untrusted comment: verify with openbsd-71-base.pub RWR2eHwZTOEiTaqC4UKeBekmp/ISPd5T+9jazUw0nEyfhx85Mbs8dsMagubYWka6+p3/JGVALwtVwEwTDDyUpiOFSObpJJjUEwE= OpenBSD 7.1 errata 006, July 24, 2022: Input validation failures in the X server request parsing code can lead to out of bounds memory accesses for authorized clients. Apply by doing: signify -Vep /etc/signify/openbsd-71-base.pub -x 006_xserver.patch.sig \ -m - | (cd /usr/xenocara && patch -p0) And then compile and rebuild the X server cd /usr/xenocara/xserver make -f Makefile.bsd-wrapper obj make -f Makefile.bsd-wrapper build Index: xserver/xkb/xkb.c =================================================================== RCS file: /cvs/xenocara/xserver/xkb/xkb.c,v retrieving revision 1.22 diff -u -p -r1.22 xkb.c --- xserver/xkb/xkb.c 20 Feb 2022 17:41:36 -0000 1.22 +++ xserver/xkb/xkb.c 13 Jul 2022 20:38:58 -0000 @@ -2511,16 +2511,15 @@ _XkbSetMapChecks(ClientPtr client, Devic } } - if (!(req->present & XkbKeyTypesMask)) { - nTypes = xkb->map->num_types; - } - else if (!CheckKeyTypes(client, xkb, req, (xkbKeyTypeWireDesc **) &values, - &nTypes, mapWidths, doswap)) { + /* nTypes/mapWidths/symsPerKey must be filled for further tests below, + * regardless of client-side flags */ + + if (!CheckKeyTypes(client, xkb, req, (xkbKeyTypeWireDesc **) &values, + &nTypes, mapWidths, doswap)) { client->errorValue = nTypes; return BadValue; } - /* symsPerKey/mapWidths must be filled regardless of client-side flags */ map = &xkb->map->key_sym_map[xkb->min_key_code]; for (i = xkb->min_key_code; i < xkb->max_key_code; i++, map++) { register int g, ng, w; @@ -5157,7 +5156,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 +5167,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 +5272,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 +5283,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 +5297,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 +5308,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 +5344,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 +5372,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); @@ -5369,16 +5386,19 @@ _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; + if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1)) + return BadLength; + 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,14 +5410,14 @@ _CheckSetSections(XkbGeometryPtr geom, return BadMatch; } } - rWire = (xkbRowWireDesc *) &kWire[rWire->nKeys]; + rWire = (xkbRowWireDesc *)kWire; } wire = (char *) rWire; if (sWire->nDoodads > 0) { 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 +5426,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 +5460,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,21 +5473,27 @@ _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++) { - pt->x = ptWire[p].x; - pt->y = ptWire[p].y; + 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) { 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]; @@ -5561,12 +5590,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; @@ -6551,7 +6583,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; @@ -6559,6 +6592,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); @@ -6586,6 +6624,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); } @@ -6597,6 +6640,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); @@ -6648,11 +6695,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); @@ -6671,10 +6713,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; @@ -6692,10 +6730,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; @@ -6731,13 +6765,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; @@ -6748,13 +6786,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; } @@ -6765,8 +6803,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; @@ -6790,8 +6828,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;