Module Name: xsrc Committed By: mrg Date: Sat Dec 5 23:36:45 UTC 2020
Modified Files: xsrc/external/mit/xorg-server.old/dist/xkb: xkb.c Log Message: merge security fixes for xkb, as found in these xserver gitlab commits: 270e439739e023463e7e0719a4eede69d45f7a3f - xkb: only swap once in XkbSetMap 446ff2d3177087b8173fa779fa5b77a2a128988b - Check SetMap request length carefully 87c64fc5b0db9f62f4e361444f4b60501ebf67b9 - Fix XkbSetDeviceInfo() and SetDeviceIndicators() heap overflows de940e06f8733d87bbb857aef85d830053442cfe - xkb: fix key type index check in _XkbSetMapChecks f7cd1276bbd4fe3a9700096dec33b52b8440788d - Correct bounds checking in XkbSetNames() i haven't tested these run OK, and it was a 33 out of 34 hunks did not apply cleanly, but they merge was still largely the same (patch failed due to whitespace changes mostly), and i am able to build-test successfully. To generate a diff of this commit: cvs rdiff -u -r1.1.1.1 -r1.2 xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c diff -u xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c:1.1.1.1 xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c:1.2 --- xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c:1.1.1.1 Thu Jun 9 09:08:01 2016 +++ xsrc/external/mit/xorg-server.old/dist/xkb/xkb.c Sat Dec 5 23:36:45 2020 @@ -151,6 +151,19 @@ static RESTYPE RT_XKBCLIENT; #define CHK_REQ_KEY_RANGE(err,first,num,r) \ CHK_REQ_KEY_RANGE2(err,first,num,r,client->errorValue,BadValue) +static Bool +_XkbCheckRequestBounds(ClientPtr client, void *stuff, void *from, void *to) { + char *cstuff = (char *)stuff; + char *cfrom = (char *)from; + char *cto = (char *)to; + + return cfrom < cto && + cfrom >= cstuff && + cfrom < cstuff + ((size_t)client->req_len << 2) && + cto >= cstuff && + cto <= cstuff + ((size_t)client->req_len << 2); +} + /***====================================================================***/ int @@ -1550,7 +1563,8 @@ CheckKeyTypes( ClientPtr client, xkbSetMapReq * req, xkbKeyTypeWireDesc **wireRtrn, int * nMapsRtrn, - CARD8 * mapWidthRtrn) + CARD8 * mapWidthRtrn, + Bool doswap) { unsigned nMaps; register unsigned i,n; @@ -1588,7 +1602,7 @@ register xkbKeyTypeWireDesc *wire = *wir } for (i=0;i<req->nTypes;i++) { unsigned width; - if (client->swapped) { + if (client->swapped && doswap) { register int s; swaps(&wire->virtualMods,s); } @@ -1615,7 +1629,7 @@ register xkbKeyTypeWireDesc *wire = *wir mapWire= (xkbKTSetMapEntryWireDesc *)&wire[1]; preWire= (xkbModsWireDesc *)&mapWire[wire->nMapEntries]; for (n=0;n<wire->nMapEntries;n++) { - if (client->swapped) { + if (client->swapped && doswap) { register int s; swaps(&mapWire[n].virtualMods,s); } @@ -1634,7 +1648,7 @@ register xkbKeyTypeWireDesc *wire = *wir return 0; } if (wire->preserve) { - if (client->swapped) { + if (client->swapped && doswap) { register int s; swaps(&preWire[n].virtualMods,s); } @@ -1673,7 +1687,8 @@ CheckKeySyms( ClientPtr client, CARD8 * mapWidths, CARD16 * symsPerKey, xkbSymMapWireDesc ** wireRtrn, - int * errorRtrn) + int * errorRtrn, + Bool doswap) { register unsigned i; XkbSymMapPtr map; @@ -1685,7 +1700,7 @@ xkbSymMapWireDesc* wire = *wireRtrn; for (i=0;i<req->nKeySyms;i++) { KeySym *pSyms; register unsigned nG; - if (client->swapped) { + if (client->swapped && doswap) { swaps(&wire->nSyms,nG); } nG = XkbNumGroups(wire->groupInfo); @@ -2322,13 +2337,99 @@ XkbServerMapPtr srv = xkbi->desc->serve } return (char *)wire; } + +#define _add_check_len(new) \ + if (len > UINT32_MAX - (new) || len > req_len - (new)) goto bad; \ + else len += new + +/** + * Check the length of the SetMap request + */ +static int +_XkbSetMapCheckLength(xkbSetMapReq *req) +{ + size_t len = sz_xkbSetMapReq, req_len = req->length << 2; + xkbKeyTypeWireDesc *keytype; + xkbSymMapWireDesc *symmap; + BOOL preserve; + int i, map_count, nSyms; + + if (req_len < len) + goto bad; + /* types */ + if (req->present & XkbKeyTypesMask) { + keytype = (xkbKeyTypeWireDesc *)(req + 1); + for (i = 0; i < req->nTypes; i++) { + _add_check_len(XkbPaddedSize(sz_xkbKeyTypeWireDesc)); + if (req->flags & XkbSetMapResizeTypes) { + _add_check_len(keytype->nMapEntries + * sz_xkbKTSetMapEntryWireDesc); + preserve = keytype->preserve; + map_count = keytype->nMapEntries; + if (preserve) { + _add_check_len(map_count * sz_xkbModsWireDesc); + } + keytype += 1; + keytype = (xkbKeyTypeWireDesc *) + ((xkbKTSetMapEntryWireDesc *)keytype + map_count); + if (preserve) + keytype = (xkbKeyTypeWireDesc *) + ((xkbModsWireDesc *)keytype + map_count); + } + } + } + /* syms */ + if (req->present & XkbKeySymsMask) { + symmap = (xkbSymMapWireDesc *)((char *)req + len); + for (i = 0; i < req->nKeySyms; i++) { + _add_check_len(sz_xkbSymMapWireDesc); + nSyms = symmap->nSyms; + _add_check_len(nSyms*sizeof(CARD32)); + symmap += 1; + symmap = (xkbSymMapWireDesc *)((CARD32 *)symmap + nSyms); + } + } + /* actions */ + if (req->present & XkbKeyActionsMask) { + _add_check_len(req->totalActs * sz_xkbActionWireDesc + + XkbPaddedSize(req->nKeyActs)); + } + /* behaviours */ + if (req->present & XkbKeyBehaviorsMask) { + _add_check_len(req->totalKeyBehaviors * sz_xkbBehaviorWireDesc); + } + /* vmods */ + if (req->present & XkbVirtualModsMask) { + _add_check_len(XkbPaddedSize(Ones(req->virtualMods))); + } + /* explicit */ + if (req->present & XkbExplicitComponentsMask) { + /* two bytes per non-zero explicit componen */ + _add_check_len(XkbPaddedSize(req->totalKeyExplicit * sizeof(CARD16))); + } + /* modmap */ + if (req->present & XkbModifierMapMask) { + /* two bytes per non-zero modmap component */ + _add_check_len(XkbPaddedSize(req->totalModMapKeys * sizeof(CARD16))); + } + /* vmodmap */ + if (req->present & XkbVirtualModMapMask) { + _add_check_len(req->totalVModMapKeys * sz_xkbVModMapWireDesc); + } + if (len == req_len) + return Success; +bad: + ErrorF("[xkb] BOGUS LENGTH in SetMap: expected %ld got %ld\n", + len, req_len); + return BadLength; +} /** * Check if the given request can be applied to the given device but don't - * actually do anything.. + * actually do anything, except swap values when client->swapped and doswap are both true. */ static int -_XkbSetMapChecks(ClientPtr client, DeviceIntPtr dev, xkbSetMapReq *req, char* values) +_XkbSetMapChecks(ClientPtr client, DeviceIntPtr dev, xkbSetMapReq *req, char* values, Bool doswap) { XkbSrvInfoPtr xkbi; XkbDescPtr xkb; @@ -2362,10 +2463,13 @@ _XkbSetMapChecks(ClientPtr client, Devic if ((req->present & XkbKeyTypesMask) && (!CheckKeyTypes(client,xkb,req,(xkbKeyTypeWireDesc **)&values, - &nTypes,mapWidths))) { + &nTypes,mapWidths, doswap))) { client->errorValue = nTypes; return BadValue; } + else { + nTypes = xkb->map->num_types; + } /* symsPerKey/mapWidths must be filled regardless of client-side flags */ map = &xkb->map->key_sym_map[xkb->min_key_code]; @@ -2375,7 +2479,7 @@ _XkbSetMapChecks(ClientPtr client, Devic for (w=g=0;g<ng;g++) { if (map->kt_index[g]>=(unsigned)nTypes) { client->errorValue = _XkbErrCode4(0x13,i,g,map->kt_index[g]); - return 0; + return BadValue; } if (mapWidths[map->kt_index[g]]>w) w= mapWidths[map->kt_index[g]]; @@ -2385,7 +2489,7 @@ _XkbSetMapChecks(ClientPtr client, Devic if ((req->present & XkbKeySymsMask) && (!CheckKeySyms(client,xkb,req,nTypes,mapWidths,symsPerKey, - (xkbSymMapWireDesc **)&values,&error))) { + (xkbSymMapWireDesc **)&values,&error, doswap))) { client->errorValue = error; return BadValue; } @@ -2554,12 +2658,17 @@ ProcXkbSetMap(ClientPtr client) CHK_KBD_DEVICE(dev, stuff->deviceSpec, client, DixManageAccess); CHK_MASK_LEGAL(0x01,stuff->present,XkbAllMapComponentsMask); + /* first verify the request length carefully */ + rc = _XkbSetMapCheckLength(stuff); + if (rc != Success) + return rc; + tmp = (char *)&stuff[1]; /* Check if we can to the SetMap on the requested device. If this succeeds, do the same thing for all extension devices (if needed). If any of them fails, fail. */ - rc = _XkbSetMapChecks(client, dev, stuff, tmp); + rc = _XkbSetMapChecks(client, dev, stuff, tmp, TRUE); if (rc != Success) return rc; @@ -2574,7 +2683,7 @@ ProcXkbSetMap(ClientPtr client) rc = XaceHook(XACE_DEVICE_ACCESS, client, other, DixManageAccess); if (rc == Success) { - rc = _XkbSetMapChecks(client, other, stuff, tmp); + rc = _XkbSetMapChecks(client, other, stuff, tmp, FALSE); if (rc != Success) return rc; } @@ -3914,6 +4023,8 @@ _XkbSetNamesCheck(ClientPtr client, Devi client->errorValue = _XkbErrCode2(0x04,stuff->firstType); return BadAccess; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + stuff->nTypes)) + return BadLength; old= tmp; tmp= _XkbCheckAtoms(tmp,stuff->nTypes,client->swapped,&bad); if (!tmp) { @@ -3941,6 +4052,8 @@ _XkbSetNamesCheck(ClientPtr client, Devi } width = (CARD8 *)tmp; tmp= (CARD32 *)(((char *)tmp)+XkbPaddedSize(stuff->nKTLevels)); + if (!_XkbCheckRequestBounds(client, stuff, width, tmp)) + return BadLength; type = &xkb->map->types[stuff->firstKTLevel]; for (i=0;i<stuff->nKTLevels;i++,type++) { if (width[i]==0) @@ -3950,6 +4063,8 @@ _XkbSetNamesCheck(ClientPtr client, Devi type->num_levels,width[i]); return BadMatch; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + width[i])) + return BadLength; tmp= _XkbCheckAtoms(tmp,width[i],client->swapped,&bad); if (!tmp) { client->errorValue= bad; @@ -3962,6 +4077,9 @@ _XkbSetNamesCheck(ClientPtr client, Devi client->errorValue= 0x08; return BadMatch; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, + tmp + Ones(stuff->indicators))) + return BadLength; tmp= _XkbCheckMaskedAtoms(tmp,XkbNumIndicators,stuff->indicators, client->swapped,&bad); if (!tmp) { @@ -3974,6 +4092,9 @@ _XkbSetNamesCheck(ClientPtr client, Devi client->errorValue= 0x09; return BadMatch; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, + tmp + Ones(stuff->virtualMods))) + return BadLength; tmp= _XkbCheckMaskedAtoms(tmp,XkbNumVirtualMods, (CARD32)stuff->virtualMods, client->swapped,&bad); @@ -3987,6 +4108,9 @@ _XkbSetNamesCheck(ClientPtr client, Devi client->errorValue= 0x0a; return BadMatch; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, + tmp + Ones(stuff->groupNames))) + return BadLength; tmp= _XkbCheckMaskedAtoms(tmp,XkbNumKbdGroups, (CARD32)stuff->groupNames, client->swapped,&bad); @@ -4007,9 +4131,14 @@ _XkbSetNamesCheck(ClientPtr client, Devi stuff->firstKey,stuff->nKeys); return BadValue; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + stuff->nKeys)) + return BadLength; tmp+= stuff->nKeys; } if ((stuff->which&XkbKeyAliasesMask)&&(stuff->nKeyAliases>0)) { + if (!_XkbCheckRequestBounds(client, stuff, tmp, + tmp + (stuff->nKeyAliases * 2))) + return BadLength; tmp+= stuff->nKeyAliases*2; } if (stuff->which&XkbRGNamesMask) { @@ -4017,6 +4146,9 @@ _XkbSetNamesCheck(ClientPtr client, Devi client->errorValue= _XkbErrCode2(0x0d,stuff->nRadioGroups); return BadValue; } + if (!_XkbCheckRequestBounds(client, stuff, tmp, + tmp + stuff->nRadioGroups)) + return BadLength; tmp= _XkbCheckAtoms(tmp,stuff->nRadioGroups,client->swapped,&bad); if (!tmp) { client->errorValue= bad; @@ -4208,6 +4340,8 @@ ProcXkbSetNames(ClientPtr client) /* check device-independent stuff */ tmp = (CARD32 *)&stuff[1]; + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbKeycodesNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -4215,6 +4349,8 @@ ProcXkbSetNames(ClientPtr client) return BadAtom; } } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbGeometryNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -4222,6 +4358,8 @@ ProcXkbSetNames(ClientPtr client) return BadAtom; } } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbSymbolsNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -4229,6 +4367,8 @@ ProcXkbSetNames(ClientPtr client) return BadAtom; } } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbPhysSymbolsNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -4236,6 +4376,8 @@ ProcXkbSetNames(ClientPtr client) return BadAtom; } } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbTypesNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -4243,6 +4385,8 @@ ProcXkbSetNames(ClientPtr client) return BadAtom; } } + if (!_XkbCheckRequestBounds(client, stuff, tmp, tmp + 1)) + return BadLength; if (stuff->which&XkbCompatNameMask) { tmp= _XkbCheckAtoms(tmp,1,client->swapped,&bad); if (!tmp) { @@ -6350,7 +6494,8 @@ SetDeviceIndicators( char * wire, int num, int * status_rtrn, ClientPtr client, - xkbExtensionDeviceNotify *ev) + xkbExtensionDeviceNotify *ev, + xkbSetDeviceInfoReq *stuff) { xkbDeviceLedsWireDesc * ledWire; int i; @@ -6371,6 +6516,11 @@ DeviceIntPtr kbd; 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); @@ -6389,10 +6539,14 @@ DeviceIntPtr kbd; memset((char *)sli->names, 0, XkbNumIndicators*sizeof(Atom)); for (n=0,bit=1;n<XkbNumIndicators;n++,bit<<=1) { if (ledWire->namesPresent&bit) { - sli->names[n]= (Atom)*atomWire; - if (sli->names[n]==None) + 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; - atomWire++; + atomWire++; } } } @@ -6405,6 +6559,10 @@ DeviceIntPtr kbd; 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; @@ -6495,7 +6653,11 @@ _XkbSetDeviceInfoCheck(ClientPtr client, return BadAlloc; dev->button->xkb_acts= acts; } + 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; @@ -6513,7 +6675,7 @@ _XkbSetDeviceInfoCheck(ClientPtr client, if (stuff->change&XkbXI_IndicatorsMask) { int status= Success; wire= SetDeviceIndicators(wire,dev,stuff->change, - stuff->nDeviceLedFBs, &status,client,&ed); + stuff->nDeviceLedFBs, &status,client,&ed, stuff); if (status!=Success) return status; }