diff -u xorg-server-1.20.13/debian/changelog xorg-server-1.20.13/debian/changelog --- xorg-server-1.20.13/debian/changelog +++ xorg-server-1.20.13/debian/changelog @@ -1,3 +1,18 @@ +xorg-server (2:1.20.13-1ubuntu1~20.04.3) focal-security; urgency=medium + + * SECURITY UPDATE: ProcXkbSetGeometry Out-Of-Bounds Access + - debian/patches/CVE-2022-2319-pre1.patch: switch to array index loops + to moving pointers in xkb/xkb.c. + - debian/patches/CVE-2022-2319.patch: add request length validation for + XkbSetGeometry in xkb/xkb.c. + - CVE-2022-2319 + * SECURITY UPDATE: ProcXkbSetDeviceInfo Out-Of-Bounds Access + - debian/patches/CVE-2022-2320.patch: swap XkbSetDeviceInfo and + XkbSetDeviceInfoCheck in xkb/xkb.c. + - CVE-2022-2320 + + -- Marc Deslauriers Wed, 06 Jul 2022 09:53:24 -0400 + xorg-server (2:1.20.13-1ubuntu1~20.04.2) focal-security; urgency=medium * SECURITY UPDATE: SProcRenderCompositeGlyphs out-of-bounds access diff -u xorg-server-1.20.13/debian/patches/series xorg-server-1.20.13/debian/patches/series --- xorg-server-1.20.13/debian/patches/series +++ xorg-server-1.20.13/debian/patches/series @@ -36,0 +37,3 @@ +CVE-2022-2319-pre1.patch +CVE-2022-2319.patch +CVE-2022-2320.patch only in patch2: unchanged: --- xorg-server-1.20.13.orig/debian/patches/CVE-2022-2319-pre1.patch +++ xorg-server-1.20.13/debian/patches/CVE-2022-2319-pre1.patch @@ -0,0 +1,70 @@ +From de1e70e6d179361291312d2c2effb246eedbdd88 Mon Sep 17 00:00:00 2001 +From: Peter Hutterer +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 +Reviewed-by: Olivier Fourdan +--- + 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 +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 +--- + 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 +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 +--- + 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;