Module Name: xsrc
Committed By: martin
Date: Mon Dec 7 19:29:26 UTC 2020
Modified Files:
xsrc/external/mit/xorg-server.old/dist/xkb [netbsd-8]: xkb.c
Log Message:
Pull up following revision(s) (requested by mrg in ticket #1628):
external/mit/xorg-server.old/dist/xkb/xkb.c: revision 1.2
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.1.1.1.2.1 \
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.1.1.1.2.1
--- 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 Mon Dec 7 19:29:26 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;
}
