extern/KasmVNC
1
0
Fork 0
mirror of https://github.com/kasmtech/KasmVNC.git synced 2024-11-21 15:43:28 +01:00
Code Issues Packages Projects Releases Wiki Activity

adding CVE patches for out of bounds memory access

Browse Source
This commit is contained in:
ryan.kuba 2022-07-14 09:33:09 -04:00
parent ba902f8194
commit a4ac7bee16
3 changed files with 864 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
builder/build.sh
View File

@ -46,9 +46,13 @@ cd unix/xserver
patch -Np1 -i ../xserver${XORG_PATCH}.patch
case "$XORG_VER" in
1.20.*)
patch -s -p0 < ../CVE-2022-2320-v1.20.patch
if [ -f ../xserver120.7.patch ]; then
patch -Np1 -i ../xserver120.7.patch
fi ;;
1.19.*)
patch -s -p0 < ../CVE-2022-2320-v1.19.patch
;;
esac
autoreconf -i

430
unix/CVE-2022-2320-v1.19.patch Normal file
View File

@ -0,0 +1,430 @@
@@ -0,0 +1,70 @@
+From de1e70e6d179361291312d2c2effb246eedbdd88 Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 12:40:47 +1000
+Subject: [PATCH] xkb: switch to array index loops to moving pointers
+
+Most similar loops here use a pointer that advances with each loop
+iteration, let's do the same here for consistency.
+
+No functional changes.
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
+---
+ xkb/xkb.c | 20 ++++++++++----------
+ 1 file changed, 10 insertions(+), 10 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5335,16 +5335,16 @@ _CheckSetSections(XkbGeometryPtr geom,
+ row->left = rWire->left;
+ row->vertical = rWire->vertical;
+ kWire = (xkbKeyWireDesc *) &rWire[1];
+- for (k = 0; k < rWire->nKeys; k++) {
++ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+- memcpy(key->name.name, kWire[k].name, XkbKeyNameLength);
+- key->gap = kWire[k].gap;
+- key->shape_ndx = kWire[k].shapeNdx;
+- key->color_ndx = kWire[k].colorNdx;
++ memcpy(key->name.name, kWire->name, XkbKeyNameLength);
++ key->gap = kWire->gap;
++ key->shape_ndx = kWire->shapeNdx;
++ key->color_ndx = kWire->colorNdx;
+ if (key->shape_ndx >= geom->num_shapes) {
+ client->errorValue = _XkbErrCode3(0x10, key->shape_ndx,
+ geom->num_shapes);
+@@ -5356,7 +5356,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ return BadMatch;
+ }
+ }
+- rWire = (xkbRowWireDesc *) &kWire[rWire->nKeys];
++ rWire = (xkbRowWireDesc *)kWire;
+ }
+ wire = (char *) rWire;
+ if (sWire->nDoodads > 0) {
+@@ -5421,16 +5421,16 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+- for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++) {
+- pt->x = ptWire[p].x;
+- pt->y = ptWire[p].y;
++ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++, ptWire++) {
++ pt->x = ptWire->x;
++ pt->y = ptWire->y;
+ if (client->swapped) {
+ swaps(&pt->x);
+ swaps(&pt->y);
+ }
+ }
+ ol->num_points = olWire->nPoints;
+- olWire = (xkbOutlineWireDesc *) (&ptWire[olWire->nPoints]);
++ olWire = (xkbOutlineWireDesc *)ptWire;
+ }
+ if (shapeWire->primaryNdx != XkbNoShape)
+ shape->primary = &shape->outlines[shapeWire->primaryNdx];
only in patch2:
unchanged:
--- xorg-server-1.19.6.orig/debian/patches/CVE-2022-2319.patch
+++ xorg-server-1.19.6/debian/patches/CVE-2022-2319.patch
@@ -0,0 +1,176 @@
+From 1ff4ec6c6cc3d5acf47ee95ef1c5fb0b91d5cb08 Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 11:11:06 +1000
+Subject: [PATCH xserver] xkb: add request length validation for XkbSetGeometry
+
+No validation of the various fields on that report were done, so a
+malicious client could send a short request that claims it had N
+sections, or rows, or keys, and the server would process the request for
+N sections, running out of bounds of the actual request data.
+
+Fix this by adding size checks to ensure our data is valid.
+
+ZDI-CAN 16062
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+---
+ xkb/xkb.c | 43 ++++++++++++++++++++++++++++++++++++++-----
+ 1 file changed, 38 insertions(+), 5 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5123,7 +5123,7 @@ _GetCountedString(char **wire_inout, Cli
+ }
+
+ static Status
+-_CheckSetDoodad(char **wire_inout,
++_CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ char *wire;
+@@ -5134,6 +5134,9 @@ _CheckSetDoodad(char **wire_inout,
+ Status status;
+
+ dWire = (xkbDoodadWireDesc *) (*wire_inout);
++ if (!_XkbCheckRequestBounds(client, req, dWire, dWire + 1))
++ return BadLength;
++
+ any = dWire->any;
+ wire = (char *) &dWire[1];
+ if (client->swapped) {
+@@ -5236,7 +5239,7 @@ _CheckSetDoodad(char **wire_inout,
+ }
+
+ static Status
+-_CheckSetOverlay(char **wire_inout,
++_CheckSetOverlay(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ register int r;
+@@ -5247,6 +5250,9 @@ _CheckSetOverlay(char **wire_inout,
+
+ wire = *wire_inout;
+ olWire = (xkbOverlayWireDesc *) wire;
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&olWire->name);
+ }
+@@ -5258,6 +5264,9 @@ _CheckSetOverlay(char **wire_inout,
+ xkbOverlayKeyWireDesc *kWire;
+ XkbOverlayRowPtr row;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (rWire->rowUnder > section->num_rows) {
+ client->errorValue = _XkbErrCode4(0x20, r, section->num_rows,
+ rWire->rowUnder);
+@@ -5266,6 +5275,9 @@ _CheckSetOverlay(char **wire_inout,
+ row = XkbAddGeomOverlayRow(ol, rWire->rowUnder, rWire->nKeys);
+ kWire = (xkbOverlayKeyWireDesc *) &rWire[1];
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ if (XkbAddGeomOverlayKey(ol, row,
+ (char *) kWire->over,
+ (char *) kWire->under) == NULL) {
+@@ -5299,6 +5311,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int r;
+ xkbRowWireDesc *rWire;
+
++ if (!_XkbCheckRequestBounds(client, req, sWire, sWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&sWire->name);
+ swaps(&sWire->top);
+@@ -5324,6 +5339,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ XkbRowPtr row;
+ xkbKeyWireDesc *kWire;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swaps(&rWire->top);
+ swaps(&rWire->left);
+@@ -5338,6 +5356,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+@@ -5363,7 +5384,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int d;
+
+ for (d = 0; d < sWire->nDoodads; d++) {
+- status = _CheckSetDoodad(&wire, geom, section, client);
++ status = _CheckSetDoodad(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5372,7 +5393,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int o;
+
+ for (o = 0; o < sWire->nOverlays; o++) {
+- status = _CheckSetOverlay(&wire, geom, section, client);
++ status = _CheckSetOverlay(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5406,6 +5427,9 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ xkbOutlineWireDesc *olWire;
+ XkbOutlinePtr ol;
+
++ if (!_XkbCheckRequestBounds(client, req, shapeWire, shapeWire + 1))
++ return BadLength;
++
+ shape =
+ XkbAddGeomShape(geom, shapeWire->name, shapeWire->nOutlines);
+ if (!shape)
+@@ -5416,12 +5440,18 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ XkbPointPtr pt;
+ xkbPointWireDesc *ptWire;
+
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ ol = XkbAddGeomOutline(shape, olWire->nPoints);
+ if (!ol)
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++, ptWire++) {
++ if (!_XkbCheckRequestBounds(client, req, ptWire, ptWire + 1))
++ return BadLength;
++
+ pt->x = ptWire->x;
+ pt->y = ptWire->y;
+ if (client->swapped) {
+@@ -5527,12 +5557,15 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSe
+ return status;
+
+ for (i = 0; i < req->nDoodads; i++) {
+- status = _CheckSetDoodad(&wire, geom, NULL, client);
++ status = _CheckSetDoodad(&wire, req, geom, NULL, client);
+ if (status != Success)
+ return status;
+ }
+
+ for (i = 0; i < req->nKeyAliases; i++) {
++ if (!_XkbCheckRequestBounds(client, req, wire, wire + XkbKeyNameLength))
++ return BadLength;
++
+ if (XkbAddGeomKeyAlias(geom, &wire[XkbKeyNameLength], wire) == NULL)
+ return BadAlloc;
+ wire += 2 * XkbKeyNameLength;
only in patch2:
unchanged:
--- xorg-server-1.19.6.orig/debian/patches/CVE-2022-2320.patch
+++ xorg-server-1.19.6/debian/patches/CVE-2022-2320.patch
@@ -0,0 +1,173 @@
+From 4e597615d43a1946942c8a004f9b9079b12e3b8a Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 09:50:41 +1000
+Subject: [PATCH xserver] xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck
+
+XKB often uses a FooCheck and Foo function pair, the former is supposed
+to check all values in the request and error out on BadLength,
+BadValue, etc. The latter is then called once we're confident the values
+are good (they may still fail on an individual device, but that's a
+different topic).
+
+In the case of XkbSetDeviceInfo, those functions were incorrectly
+named, with XkbSetDeviceInfo ending up as the checker function and
+XkbSetDeviceInfoCheck as the setter function. As a result, the setter
+function was called before the checker function, accessing request
+data and modifying device state before we ensured that the data is
+valid.
+
+In particular, the setter function relied on values being already
+byte-swapped. This in turn could lead to potential OOB memory access.
+
+Fix this by correctly naming the functions and moving the length checks
+over to the checker function. These were added in 87c64fc5b0 to the
+wrong function, probably due to the incorrect naming.
+
+Fixes ZDI-CAN 16070
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Introduced in c06e27b2f6fd9f7b9f827623a48876a225264132
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+---
+ xkb/xkb.c | 46 +++++++++++++++++++++++++---------------------
+ 1 file changed, 25 insertions(+), 21 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -6550,7 +6550,8 @@ ProcXkbGetDeviceInfo(ClientPtr client)
+ static char *
+ CheckSetDeviceIndicators(char *wire,
+ DeviceIntPtr dev,
+- int num, int *status_rtrn, ClientPtr client)
++ int num, int *status_rtrn, ClientPtr client,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ xkbDeviceLedsWireDesc *ledWire;
+ int i;
+@@ -6558,6 +6559,11 @@ CheckSetDeviceIndicators(char *wire,
+
+ ledWire = (xkbDeviceLedsWireDesc *) wire;
+ for (i = 0; i < num; i++) {
++ if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) ledWire;
++ }
++
+ if (client->swapped) {
+ swaps(&ledWire->ledClass);
+ swaps(&ledWire->ledID);
+@@ -6585,6 +6591,11 @@ CheckSetDeviceIndicators(char *wire,
+ atomWire = (CARD32 *) &ledWire[1];
+ if (nNames > 0) {
+ for (n = 0; n < nNames; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) atomWire;
++ }
++
+ if (client->swapped) {
+ swapl(atomWire);
+ }
+@@ -6596,6 +6607,10 @@ CheckSetDeviceIndicators(char *wire,
+ mapWire = (xkbIndicatorMapWireDesc *) atomWire;
+ if (nMaps > 0) {
+ for (n = 0; n < nMaps; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) mapWire;
++ }
+ if (client->swapped) {
+ swaps(&mapWire->virtualMods);
+ swapl(&mapWire->ctrls);
+@@ -6647,11 +6662,6 @@ SetDeviceIndicators(char *wire,
+ xkbIndicatorMapWireDesc *mapWire;
+ XkbSrvLedInfoPtr sli;
+
+- if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) ledWire;
+- }
+-
+ namec = mapc = statec = 0;
+ sli = XkbFindSrvLedInfo(dev, ledWire->ledClass, ledWire->ledID,
+ XkbXI_IndicatorMapsMask);
+@@ -6670,10 +6680,6 @@ SetDeviceIndicators(char *wire,
+ memset((char *) sli->names, 0, XkbNumIndicators * sizeof(Atom));
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->namesPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) atomWire;
+- }
+ sli->names[n] = (Atom) *atomWire;
+ if (sli->names[n] == None)
+ ledWire->namesPresent &= ~bit;
+@@ -6691,10 +6697,6 @@ SetDeviceIndicators(char *wire,
+ if (ledWire->mapsPresent) {
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->mapsPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) mapWire;
+- }
+ sli->maps[n].flags = mapWire->flags;
+ sli->maps[n].which_groups = mapWire->whichGroups;
+ sli->maps[n].groups = mapWire->groups;
+@@ -6730,13 +6732,17 @@ SetDeviceIndicators(char *wire,
+ }
+
+ static int
+-_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+
+ wire = (char *) &stuff[1];
+ if (stuff->change & XkbXI_ButtonActionsMask) {
++ int sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
++ if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
++ return BadLength;
++
+ if (!dev->button) {
+ client->errorValue = _XkbErrCode2(XkbErr_BadClass, ButtonClass);
+ return XkbKeyboardErrorCode;
+@@ -6747,13 +6753,13 @@ _XkbSetDeviceInfo(ClientPtr client, Devi
+ dev->button->numButtons);
+ return BadMatch;
+ }
+- wire += (stuff->nBtns * SIZEOF(xkbActionWireDesc));
++ wire += sz;
+ }
+ if (stuff->change & XkbXI_IndicatorsMask) {
+ int status = Success;
+
+ wire = CheckSetDeviceIndicators(wire, dev, stuff->nDeviceLedFBs,
+- &status, client);
++ &status, client, stuff);
+ if (status != Success)
+ return status;
+ }
+@@ -6764,8 +6770,8 @@ _XkbSetDeviceInfo(ClientPtr client, Devi
+ }
+
+ static int
+-_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+- xkbSetDeviceInfoReq * stuff)
++_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+ xkbExtensionDeviceNotify ed;
+@@ -6789,8 +6795,6 @@ _XkbSetDeviceInfoCheck(ClientPtr client,
+ if (stuff->firstBtn + stuff->nBtns > nBtns)
+ return BadValue;
+ sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
+- if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
+- return BadLength;
+ memcpy((char *) &acts[stuff->firstBtn], (char *) wire, sz);
+ wire += sz;
+ ed.reason |= XkbXI_ButtonActionsMask;

430
unix/CVE-2022-2320-v1.20.patch Normal file
View File

@ -0,0 +1,430 @@
@@ -0,0 +1,70 @@
+From de1e70e6d179361291312d2c2effb246eedbdd88 Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 12:40:47 +1000
+Subject: [PATCH] xkb: switch to array index loops to moving pointers
+
+Most similar loops here use a pointer that advances with each loop
+iteration, let's do the same here for consistency.
+
+No functional changes.
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
+---
+ xkb/xkb.c | 20 ++++++++++----------
+ 1 file changed, 10 insertions(+), 10 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5369,16 +5369,16 @@ _CheckSetSections(XkbGeometryPtr geom,
+ row->left = rWire->left;
+ row->vertical = rWire->vertical;
+ kWire = (xkbKeyWireDesc *) &rWire[1];
+- for (k = 0; k < rWire->nKeys; k++) {
++ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+- memcpy(key->name.name, kWire[k].name, XkbKeyNameLength);
+- key->gap = kWire[k].gap;
+- key->shape_ndx = kWire[k].shapeNdx;
+- key->color_ndx = kWire[k].colorNdx;
++ memcpy(key->name.name, kWire->name, XkbKeyNameLength);
++ key->gap = kWire->gap;
++ key->shape_ndx = kWire->shapeNdx;
++ key->color_ndx = kWire->colorNdx;
+ if (key->shape_ndx >= geom->num_shapes) {
+ client->errorValue = _XkbErrCode3(0x10, key->shape_ndx,
+ geom->num_shapes);
+@@ -5390,7 +5390,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ return BadMatch;
+ }
+ }
+- rWire = (xkbRowWireDesc *) &kWire[rWire->nKeys];
++ rWire = (xkbRowWireDesc *)kWire;
+ }
+ wire = (char *) rWire;
+ if (sWire->nDoodads > 0) {
+@@ -5455,16 +5455,16 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+- for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++) {
+- pt->x = ptWire[p].x;
+- pt->y = ptWire[p].y;
++ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++, ptWire++) {
++ pt->x = ptWire->x;
++ pt->y = ptWire->y;
+ if (client->swapped) {
+ swaps(&pt->x);
+ swaps(&pt->y);
+ }
+ }
+ ol->num_points = olWire->nPoints;
+- olWire = (xkbOutlineWireDesc *) (&ptWire[olWire->nPoints]);
++ olWire = (xkbOutlineWireDesc *)ptWire;
+ }
+ if (shapeWire->primaryNdx != XkbNoShape)
+ shape->primary = &shape->outlines[shapeWire->primaryNdx];
only in patch2:
unchanged:
--- xorg-server-1.20.13.orig/debian/patches/CVE-2022-2319.patch
+++ xorg-server-1.20.13/debian/patches/CVE-2022-2319.patch
@@ -0,0 +1,176 @@
+From 1ff4ec6c6cc3d5acf47ee95ef1c5fb0b91d5cb08 Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 11:11:06 +1000
+Subject: [PATCH xserver] xkb: add request length validation for XkbSetGeometry
+
+No validation of the various fields on that report were done, so a
+malicious client could send a short request that claims it had N
+sections, or rows, or keys, and the server would process the request for
+N sections, running out of bounds of the actual request data.
+
+Fix this by adding size checks to ensure our data is valid.
+
+ZDI-CAN 16062
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+---
+ xkb/xkb.c | 43 ++++++++++++++++++++++++++++++++++++++-----
+ 1 file changed, 38 insertions(+), 5 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -5157,7 +5157,7 @@ _GetCountedString(char **wire_inout, Cli
+ }
+
+ static Status
+-_CheckSetDoodad(char **wire_inout,
++_CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ char *wire;
+@@ -5168,6 +5168,9 @@ _CheckSetDoodad(char **wire_inout,
+ Status status;
+
+ dWire = (xkbDoodadWireDesc *) (*wire_inout);
++ if (!_XkbCheckRequestBounds(client, req, dWire, dWire + 1))
++ return BadLength;
++
+ any = dWire->any;
+ wire = (char *) &dWire[1];
+ if (client->swapped) {
+@@ -5270,7 +5273,7 @@ _CheckSetDoodad(char **wire_inout,
+ }
+
+ static Status
+-_CheckSetOverlay(char **wire_inout,
++_CheckSetOverlay(char **wire_inout, xkbSetGeometryReq *req,
+ XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
+ {
+ register int r;
+@@ -5281,6 +5284,9 @@ _CheckSetOverlay(char **wire_inout,
+
+ wire = *wire_inout;
+ olWire = (xkbOverlayWireDesc *) wire;
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&olWire->name);
+ }
+@@ -5292,6 +5298,9 @@ _CheckSetOverlay(char **wire_inout,
+ xkbOverlayKeyWireDesc *kWire;
+ XkbOverlayRowPtr row;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (rWire->rowUnder > section->num_rows) {
+ client->errorValue = _XkbErrCode4(0x20, r, section->num_rows,
+ rWire->rowUnder);
+@@ -5300,6 +5309,9 @@ _CheckSetOverlay(char **wire_inout,
+ row = XkbAddGeomOverlayRow(ol, rWire->rowUnder, rWire->nKeys);
+ kWire = (xkbOverlayKeyWireDesc *) &rWire[1];
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ if (XkbAddGeomOverlayKey(ol, row,
+ (char *) kWire->over,
+ (char *) kWire->under) == NULL) {
+@@ -5333,6 +5345,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int r;
+ xkbRowWireDesc *rWire;
+
++ if (!_XkbCheckRequestBounds(client, req, sWire, sWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swapl(&sWire->name);
+ swaps(&sWire->top);
+@@ -5358,6 +5373,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ XkbRowPtr row;
+ xkbKeyWireDesc *kWire;
+
++ if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
++ return BadLength;
++
+ if (client->swapped) {
+ swaps(&rWire->top);
+ swaps(&rWire->left);
+@@ -5372,6 +5390,9 @@ _CheckSetSections(XkbGeometryPtr geom,
+ for (k = 0; k < rWire->nKeys; k++, kWire++) {
+ XkbKeyPtr key;
+
++ if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
++ return BadLength;
++
+ key = XkbAddGeomKey(row);
+ if (!key)
+ return BadAlloc;
+@@ -5397,7 +5418,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int d;
+
+ for (d = 0; d < sWire->nDoodads; d++) {
+- status = _CheckSetDoodad(&wire, geom, section, client);
++ status = _CheckSetDoodad(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5406,7 +5427,7 @@ _CheckSetSections(XkbGeometryPtr geom,
+ register int o;
+
+ for (o = 0; o < sWire->nOverlays; o++) {
+- status = _CheckSetOverlay(&wire, geom, section, client);
++ status = _CheckSetOverlay(&wire, req, geom, section, client);
+ if (status != Success)
+ return status;
+ }
+@@ -5440,6 +5461,9 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ xkbOutlineWireDesc *olWire;
+ XkbOutlinePtr ol;
+
++ if (!_XkbCheckRequestBounds(client, req, shapeWire, shapeWire + 1))
++ return BadLength;
++
+ shape =
+ XkbAddGeomShape(geom, shapeWire->name, shapeWire->nOutlines);
+ if (!shape)
+@@ -5450,12 +5474,18 @@ _CheckSetShapes(XkbGeometryPtr geom,
+ XkbPointPtr pt;
+ xkbPointWireDesc *ptWire;
+
++ if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
++ return BadLength;
++
+ ol = XkbAddGeomOutline(shape, olWire->nPoints);
+ if (!ol)
+ return BadAlloc;
+ ol->corner_radius = olWire->cornerRadius;
+ ptWire = (xkbPointWireDesc *) &olWire[1];
+ for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++, ptWire++) {
++ if (!_XkbCheckRequestBounds(client, req, ptWire, ptWire + 1))
++ return BadLength;
++
+ pt->x = ptWire->x;
+ pt->y = ptWire->y;
+ if (client->swapped) {
+@@ -5561,12 +5591,15 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSe
+ return status;
+
+ for (i = 0; i < req->nDoodads; i++) {
+- status = _CheckSetDoodad(&wire, geom, NULL, client);
++ status = _CheckSetDoodad(&wire, req, geom, NULL, client);
+ if (status != Success)
+ return status;
+ }
+
+ for (i = 0; i < req->nKeyAliases; i++) {
++ if (!_XkbCheckRequestBounds(client, req, wire, wire + XkbKeyNameLength))
++ return BadLength;
++
+ if (XkbAddGeomKeyAlias(geom, &wire[XkbKeyNameLength], wire) == NULL)
+ return BadAlloc;
+ wire += 2 * XkbKeyNameLength;
only in patch2:
unchanged:
--- xorg-server-1.20.13.orig/debian/patches/CVE-2022-2320.patch
+++ xorg-server-1.20.13/debian/patches/CVE-2022-2320.patch
@@ -0,0 +1,173 @@
+From 4e597615d43a1946942c8a004f9b9079b12e3b8a Mon Sep 17 00:00:00 2001
+From: Peter Hutterer <peter.hutterer@who-t.net>
+Date: Tue, 5 Jul 2022 09:50:41 +1000
+Subject: [PATCH xserver] xkb: swap XkbSetDeviceInfo and XkbSetDeviceInfoCheck
+
+XKB often uses a FooCheck and Foo function pair, the former is supposed
+to check all values in the request and error out on BadLength,
+BadValue, etc. The latter is then called once we're confident the values
+are good (they may still fail on an individual device, but that's a
+different topic).
+
+In the case of XkbSetDeviceInfo, those functions were incorrectly
+named, with XkbSetDeviceInfo ending up as the checker function and
+XkbSetDeviceInfoCheck as the setter function. As a result, the setter
+function was called before the checker function, accessing request
+data and modifying device state before we ensured that the data is
+valid.
+
+In particular, the setter function relied on values being already
+byte-swapped. This in turn could lead to potential OOB memory access.
+
+Fix this by correctly naming the functions and moving the length checks
+over to the checker function. These were added in 87c64fc5b0 to the
+wrong function, probably due to the incorrect naming.
+
+Fixes ZDI-CAN 16070
+
+This vulnerability was discovered by:
+Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
+
+Introduced in c06e27b2f6fd9f7b9f827623a48876a225264132
+
+Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
+---
+ xkb/xkb.c | 46 +++++++++++++++++++++++++---------------------
+ 1 file changed, 25 insertions(+), 21 deletions(-)
+
+--- a/xkb/xkb.c
++++ b/xkb/xkb.c
+@@ -6584,7 +6584,8 @@ ProcXkbGetDeviceInfo(ClientPtr client)
+ static char *
+ CheckSetDeviceIndicators(char *wire,
+ DeviceIntPtr dev,
+- int num, int *status_rtrn, ClientPtr client)
++ int num, int *status_rtrn, ClientPtr client,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ xkbDeviceLedsWireDesc *ledWire;
+ int i;
+@@ -6592,6 +6593,11 @@ CheckSetDeviceIndicators(char *wire,
+
+ ledWire = (xkbDeviceLedsWireDesc *) wire;
+ for (i = 0; i < num; i++) {
++ if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) ledWire;
++ }
++
+ if (client->swapped) {
+ swaps(&ledWire->ledClass);
+ swaps(&ledWire->ledID);
+@@ -6619,6 +6625,11 @@ CheckSetDeviceIndicators(char *wire,
+ atomWire = (CARD32 *) &ledWire[1];
+ if (nNames > 0) {
+ for (n = 0; n < nNames; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) atomWire;
++ }
++
+ if (client->swapped) {
+ swapl(atomWire);
+ }
+@@ -6630,6 +6641,10 @@ CheckSetDeviceIndicators(char *wire,
+ mapWire = (xkbIndicatorMapWireDesc *) atomWire;
+ if (nMaps > 0) {
+ for (n = 0; n < nMaps; n++) {
++ if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
++ *status_rtrn = BadLength;
++ return (char *) mapWire;
++ }
+ if (client->swapped) {
+ swaps(&mapWire->virtualMods);
+ swapl(&mapWire->ctrls);
+@@ -6681,11 +6696,6 @@ SetDeviceIndicators(char *wire,
+ xkbIndicatorMapWireDesc *mapWire;
+ XkbSrvLedInfoPtr sli;
+
+- if (!_XkbCheckRequestBounds(client, stuff, ledWire, ledWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) ledWire;
+- }
+-
+ namec = mapc = statec = 0;
+ sli = XkbFindSrvLedInfo(dev, ledWire->ledClass, ledWire->ledID,
+ XkbXI_IndicatorMapsMask);
+@@ -6704,10 +6714,6 @@ SetDeviceIndicators(char *wire,
+ memset((char *) sli->names, 0, XkbNumIndicators * sizeof(Atom));
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->namesPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, atomWire, atomWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) atomWire;
+- }
+ sli->names[n] = (Atom) *atomWire;
+ if (sli->names[n] == None)
+ ledWire->namesPresent &= ~bit;
+@@ -6725,10 +6731,6 @@ SetDeviceIndicators(char *wire,
+ if (ledWire->mapsPresent) {
+ for (n = 0, bit = 1; n < XkbNumIndicators; n++, bit <<= 1) {
+ if (ledWire->mapsPresent & bit) {
+- if (!_XkbCheckRequestBounds(client, stuff, mapWire, mapWire + 1)) {
+- *status_rtrn = BadLength;
+- return (char *) mapWire;
+- }
+ sli->maps[n].flags = mapWire->flags;
+ sli->maps[n].which_groups = mapWire->whichGroups;
+ sli->maps[n].groups = mapWire->groups;
+@@ -6764,13 +6766,17 @@ SetDeviceIndicators(char *wire,
+ }
+
+ static int
+-_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+
+ wire = (char *) &stuff[1];
+ if (stuff->change & XkbXI_ButtonActionsMask) {
++ int sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
++ if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
++ return BadLength;
++
+ if (!dev->button) {
+ client->errorValue = _XkbErrCode2(XkbErr_BadClass, ButtonClass);
+ return XkbKeyboardErrorCode;
+@@ -6781,13 +6787,13 @@ _XkbSetDeviceInfo(ClientPtr client, Devi
+ dev->button->numButtons);
+ return BadMatch;
+ }
+- wire += (stuff->nBtns * SIZEOF(xkbActionWireDesc));
++ wire += sz;
+ }
+ if (stuff->change & XkbXI_IndicatorsMask) {
+ int status = Success;
+
+ wire = CheckSetDeviceIndicators(wire, dev, stuff->nDeviceLedFBs,
+- &status, client);
++ &status, client, stuff);
+ if (status != Success)
+ return status;
+ }
+@@ -6798,8 +6804,8 @@ _XkbSetDeviceInfo(ClientPtr client, Devi
+ }
+
+ static int
+-_XkbSetDeviceInfoCheck(ClientPtr client, DeviceIntPtr dev,
+- xkbSetDeviceInfoReq * stuff)
++_XkbSetDeviceInfo(ClientPtr client, DeviceIntPtr dev,
++ xkbSetDeviceInfoReq * stuff)
+ {
+ char *wire;
+ xkbExtensionDeviceNotify ed;
+@@ -6823,8 +6829,6 @@ _XkbSetDeviceInfoCheck(ClientPtr client,
+ if (stuff->firstBtn + stuff->nBtns > nBtns)
+ return BadValue;
+ sz = stuff->nBtns * SIZEOF(xkbActionWireDesc);
+- if (!_XkbCheckRequestBounds(client, stuff, wire, (char *) wire + sz))
+- return BadLength;
+ memcpy((char *) &acts[stuff->firstBtn], (char *) wire, sz);
+ wire += sz;
+ ed.reason |= XkbXI_ButtonActionsMask;