diff options
author | Peter Hutterer <peter.hutterer@who-t.net> | 2022-07-05 11:11:06 +1000 |
---|---|---|
committer | Povilas Kanapickas <povilas@radix.lt> | 2022-07-12 15:24:59 +0300 |
commit | 06b23cccb1166fcccc2f5fe7259b3ef9e1d0f32e (patch) | |
tree | 9aa0bb861813db1a4c2cec19af17316c912a8a7d | |
parent | e3a530540f2f13739b0233ec51d7a3985a7ec4be (diff) |
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, CVE-2022-2319.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 6907b6ea2b4ce949cb07271f5b678d5966d9df42)
-rw-r--r-- | xkb/xkb.c | 43 |
1 files changed, 38 insertions, 5 deletions
@@ -5156,7 +5156,7 @@ _GetCountedString(char **wire_inout, ClientPtr client, char **str) } static Status -_CheckSetDoodad(char **wire_inout, +_CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req, XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client) { char *wire; @@ -5167,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) { @@ -5269,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; @@ -5280,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); } @@ -5291,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); @@ -5299,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) { @@ -5332,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); @@ -5357,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); @@ -5371,6 +5389,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; @@ -5396,7 +5417,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; } @@ -5405,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; } @@ -5439,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) @@ -5449,12 +5473,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) { @@ -5560,12 +5590,15 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client) 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; |