xkb: Check strings length against request size

Ensure that the given strings length in an XkbSetGeometry request remain
within the limits of the size of the request.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This commit is contained in:
Olivier Fourdan 2015-01-16 08:44:45 +01:00 committed by Keith Packard
parent 81c90dc8f0
commit 20079c36cf

View File

@ -4957,25 +4957,29 @@ ProcXkbGetGeometry(ClientPtr client)
/***====================================================================***/ /***====================================================================***/
static char * static Status
_GetCountedString(char **wire_inout, Bool swap) _GetCountedString(char **wire_inout, ClientPtr client, char **str)
{ {
char *wire, *str; char *wire, *next;
CARD16 len; CARD16 len;
wire = *wire_inout; wire = *wire_inout;
len = *(CARD16 *) wire; len = *(CARD16 *) wire;
if (swap) { if (client->swapped) {
swaps(&len); swaps(&len);
} }
str = malloc(len + 1); next = wire + XkbPaddedSize(len + 2);
if (str) { /* Check we're still within the size of the request */
memcpy(str, &wire[2], len); if (client->req_len <
str[len] = '\0'; bytes_to_int32(next - (char *) client->requestBuffer))
} return BadValue;
wire += XkbPaddedSize(len + 2); *str = malloc(len + 1);
*wire_inout = wire; if (!*str)
return str; return BadAlloc;
memcpy(*str, &wire[2], len);
*(*str + len) = '\0';
*wire_inout = next;
return Success;
} }
static Status static Status
@ -4987,6 +4991,7 @@ _CheckSetDoodad(char **wire_inout,
xkbAnyDoodadWireDesc any; xkbAnyDoodadWireDesc any;
xkbTextDoodadWireDesc text; xkbTextDoodadWireDesc text;
XkbDoodadPtr doodad; XkbDoodadPtr doodad;
Status status;
dWire = (xkbDoodadWireDesc *) (*wire_inout); dWire = (xkbDoodadWireDesc *) (*wire_inout);
any = dWire->any; any = dWire->any;
@ -5036,8 +5041,14 @@ _CheckSetDoodad(char **wire_inout,
doodad->text.width = text.width; doodad->text.width = text.width;
doodad->text.height = text.height; doodad->text.height = text.height;
doodad->text.color_ndx = dWire->text.colorNdx; doodad->text.color_ndx = dWire->text.colorNdx;
doodad->text.text = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &doodad->text.text);
doodad->text.font = _GetCountedString(&wire, client->swapped); if (status != Success)
return status;
status = _GetCountedString(&wire, client, &doodad->text.font);
if (status != Success) {
free (doodad->text.text);
return status;
}
break; break;
case XkbIndicatorDoodad: case XkbIndicatorDoodad:
if (dWire->indicator.onColorNdx >= geom->num_colors) { if (dWire->indicator.onColorNdx >= geom->num_colors) {
@ -5072,7 +5083,9 @@ _CheckSetDoodad(char **wire_inout,
} }
doodad->logo.color_ndx = dWire->logo.colorNdx; doodad->logo.color_ndx = dWire->logo.colorNdx;
doodad->logo.shape_ndx = dWire->logo.shapeNdx; doodad->logo.shape_ndx = dWire->logo.shapeNdx;
doodad->logo.logo_name = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &doodad->logo.logo_name);
if (status != Success)
return status;
break; break;
default: default:
client->errorValue = _XkbErrCode2(0x4F, dWire->any.type); client->errorValue = _XkbErrCode2(0x4F, dWire->any.type);
@ -5304,18 +5317,20 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
char *wire; char *wire;
wire = (char *) &req[1]; wire = (char *) &req[1];
geom->label_font = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &geom->label_font);
if (status != Success)
return status;
for (i = 0; i < req->nProperties; i++) { for (i = 0; i < req->nProperties; i++) {
char *name, *val; char *name, *val;
name = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &name);
if (!name) if (status != Success)
return BadAlloc; return status;
val = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &val);
if (!val) { if (status != Success) {
free(name); free(name);
return BadAlloc; return status;
} }
if (XkbAddGeomProperty(geom, name, val) == NULL) { if (XkbAddGeomProperty(geom, name, val) == NULL) {
free(name); free(name);
@ -5349,9 +5364,9 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
for (i = 0; i < req->nColors; i++) { for (i = 0; i < req->nColors; i++) {
char *name; char *name;
name = _GetCountedString(&wire, client->swapped); status = _GetCountedString(&wire, client, &name);
if (!name) if (status != Success)
return BadAlloc; return status;
if (!XkbAddGeomColor(geom, name, geom->num_colors)) { if (!XkbAddGeomColor(geom, name, geom->num_colors)) {
free(name); free(name);
return BadAlloc; return BadAlloc;