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>
This commit is contained in:
Peter Hutterer 2022-07-05 11:11:06 +10:00 committed by Povilas Kanapickas
parent dd8caf39e9
commit 6907b6ea2b

View File

@ -5156,7 +5156,7 @@ _GetCountedString(char **wire_inout, ClientPtr client, char **str)
} }
static Status static Status
_CheckSetDoodad(char **wire_inout, _CheckSetDoodad(char **wire_inout, xkbSetGeometryReq *req,
XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client) XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
{ {
char *wire; char *wire;
@ -5167,6 +5167,9 @@ _CheckSetDoodad(char **wire_inout,
Status status; Status status;
dWire = (xkbDoodadWireDesc *) (*wire_inout); dWire = (xkbDoodadWireDesc *) (*wire_inout);
if (!_XkbCheckRequestBounds(client, req, dWire, dWire + 1))
return BadLength;
any = dWire->any; any = dWire->any;
wire = (char *) &dWire[1]; wire = (char *) &dWire[1];
if (client->swapped) { if (client->swapped) {
@ -5269,7 +5272,7 @@ _CheckSetDoodad(char **wire_inout,
} }
static Status static Status
_CheckSetOverlay(char **wire_inout, _CheckSetOverlay(char **wire_inout, xkbSetGeometryReq *req,
XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client) XkbGeometryPtr geom, XkbSectionPtr section, ClientPtr client)
{ {
register int r; register int r;
@ -5280,6 +5283,9 @@ _CheckSetOverlay(char **wire_inout,
wire = *wire_inout; wire = *wire_inout;
olWire = (xkbOverlayWireDesc *) wire; olWire = (xkbOverlayWireDesc *) wire;
if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
return BadLength;
if (client->swapped) { if (client->swapped) {
swapl(&olWire->name); swapl(&olWire->name);
} }
@ -5291,6 +5297,9 @@ _CheckSetOverlay(char **wire_inout,
xkbOverlayKeyWireDesc *kWire; xkbOverlayKeyWireDesc *kWire;
XkbOverlayRowPtr row; XkbOverlayRowPtr row;
if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
return BadLength;
if (rWire->rowUnder > section->num_rows) { if (rWire->rowUnder > section->num_rows) {
client->errorValue = _XkbErrCode4(0x20, r, section->num_rows, client->errorValue = _XkbErrCode4(0x20, r, section->num_rows,
rWire->rowUnder); rWire->rowUnder);
@ -5299,6 +5308,9 @@ _CheckSetOverlay(char **wire_inout,
row = XkbAddGeomOverlayRow(ol, rWire->rowUnder, rWire->nKeys); row = XkbAddGeomOverlayRow(ol, rWire->rowUnder, rWire->nKeys);
kWire = (xkbOverlayKeyWireDesc *) &rWire[1]; kWire = (xkbOverlayKeyWireDesc *) &rWire[1];
for (k = 0; k < rWire->nKeys; k++, kWire++) { for (k = 0; k < rWire->nKeys; k++, kWire++) {
if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
return BadLength;
if (XkbAddGeomOverlayKey(ol, row, if (XkbAddGeomOverlayKey(ol, row,
(char *) kWire->over, (char *) kWire->over,
(char *) kWire->under) == NULL) { (char *) kWire->under) == NULL) {
@ -5332,6 +5344,9 @@ _CheckSetSections(XkbGeometryPtr geom,
register int r; register int r;
xkbRowWireDesc *rWire; xkbRowWireDesc *rWire;
if (!_XkbCheckRequestBounds(client, req, sWire, sWire + 1))
return BadLength;
if (client->swapped) { if (client->swapped) {
swapl(&sWire->name); swapl(&sWire->name);
swaps(&sWire->top); swaps(&sWire->top);
@ -5357,6 +5372,9 @@ _CheckSetSections(XkbGeometryPtr geom,
XkbRowPtr row; XkbRowPtr row;
xkbKeyWireDesc *kWire; xkbKeyWireDesc *kWire;
if (!_XkbCheckRequestBounds(client, req, rWire, rWire + 1))
return BadLength;
if (client->swapped) { if (client->swapped) {
swaps(&rWire->top); swaps(&rWire->top);
swaps(&rWire->left); swaps(&rWire->left);
@ -5371,6 +5389,9 @@ _CheckSetSections(XkbGeometryPtr geom,
for (k = 0; k < rWire->nKeys; k++, kWire++) { for (k = 0; k < rWire->nKeys; k++, kWire++) {
XkbKeyPtr key; XkbKeyPtr key;
if (!_XkbCheckRequestBounds(client, req, kWire, kWire + 1))
return BadLength;
key = XkbAddGeomKey(row); key = XkbAddGeomKey(row);
if (!key) if (!key)
return BadAlloc; return BadAlloc;
@ -5396,7 +5417,7 @@ _CheckSetSections(XkbGeometryPtr geom,
register int d; register int d;
for (d = 0; d < sWire->nDoodads; d++) { for (d = 0; d < sWire->nDoodads; d++) {
status = _CheckSetDoodad(&wire, geom, section, client); status = _CheckSetDoodad(&wire, req, geom, section, client);
if (status != Success) if (status != Success)
return status; return status;
} }
@ -5405,7 +5426,7 @@ _CheckSetSections(XkbGeometryPtr geom,
register int o; register int o;
for (o = 0; o < sWire->nOverlays; o++) { for (o = 0; o < sWire->nOverlays; o++) {
status = _CheckSetOverlay(&wire, geom, section, client); status = _CheckSetOverlay(&wire, req, geom, section, client);
if (status != Success) if (status != Success)
return status; return status;
} }
@ -5439,6 +5460,9 @@ _CheckSetShapes(XkbGeometryPtr geom,
xkbOutlineWireDesc *olWire; xkbOutlineWireDesc *olWire;
XkbOutlinePtr ol; XkbOutlinePtr ol;
if (!_XkbCheckRequestBounds(client, req, shapeWire, shapeWire + 1))
return BadLength;
shape = shape =
XkbAddGeomShape(geom, shapeWire->name, shapeWire->nOutlines); XkbAddGeomShape(geom, shapeWire->name, shapeWire->nOutlines);
if (!shape) if (!shape)
@ -5449,12 +5473,18 @@ _CheckSetShapes(XkbGeometryPtr geom,
XkbPointPtr pt; XkbPointPtr pt;
xkbPointWireDesc *ptWire; xkbPointWireDesc *ptWire;
if (!_XkbCheckRequestBounds(client, req, olWire, olWire + 1))
return BadLength;
ol = XkbAddGeomOutline(shape, olWire->nPoints); ol = XkbAddGeomOutline(shape, olWire->nPoints);
if (!ol) if (!ol)
return BadAlloc; return BadAlloc;
ol->corner_radius = olWire->cornerRadius; ol->corner_radius = olWire->cornerRadius;
ptWire = (xkbPointWireDesc *) &olWire[1]; ptWire = (xkbPointWireDesc *) &olWire[1];
for (p = 0, pt = ol->points; p < olWire->nPoints; p++, pt++, ptWire++) { 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->x = ptWire->x;
pt->y = ptWire->y; pt->y = ptWire->y;
if (client->swapped) { if (client->swapped) {
@ -5560,12 +5590,15 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
return status; return status;
for (i = 0; i < req->nDoodads; i++) { for (i = 0; i < req->nDoodads; i++) {
status = _CheckSetDoodad(&wire, geom, NULL, client); status = _CheckSetDoodad(&wire, req, geom, NULL, client);
if (status != Success) if (status != Success)
return status; return status;
} }
for (i = 0; i < req->nKeyAliases; i++) { for (i = 0; i < req->nKeyAliases; i++) {
if (!_XkbCheckRequestBounds(client, req, wire, wire + XkbKeyNameLength))
return BadLength;
if (XkbAddGeomKeyAlias(geom, &wire[XkbKeyNameLength], wire) == NULL) if (XkbAddGeomKeyAlias(geom, &wire[XkbKeyNameLength], wire) == NULL)
return BadAlloc; return BadAlloc;
wire += 2 * XkbKeyNameLength; wire += 2 * XkbKeyNameLength;