From 1d3a1092c30af660b1366fcd344af745590aa29f Mon Sep 17 00:00:00 2001 From: Matthieu Herrb Date: Tue, 18 Aug 2020 14:46:32 +0200 Subject: Correct bounds checking in XkbSetNames() CVE-2020-14345 / ZDI 11428 This vulnerability was discovered by: Jan-Niklas Sohn working with Trend Micro Zero Day Initiative Signed-off-by: Matthieu Herrb (cherry picked from commit 11f22a3bf694d7061d552c99898d843bcdaf0cf1) --- xkb/xkb.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/xkb/xkb.c b/xkb/xkb.c index 3162574a4..2139da7ee 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -152,6 +152,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 @@ -4045,6 +4058,8 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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) { @@ -4074,6 +4089,8 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, } 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) @@ -4083,6 +4100,8 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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; @@ -4095,6 +4114,9 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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) { @@ -4107,6 +4129,9 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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); @@ -4120,6 +4145,9 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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); @@ -4141,9 +4169,14 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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) { @@ -4151,6 +4184,9 @@ _XkbSetNamesCheck(ClientPtr client, DeviceIntPtr dev, 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; @@ -4344,6 +4380,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) { @@ -4351,6 +4389,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) { @@ -4358,6 +4398,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) { @@ -4365,6 +4407,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) { @@ -4372,6 +4416,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) { @@ -4379,6 +4425,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) { -- cgit v1.2.3