(original) (raw)

untrusted comment: signature from openbsd 5.5 base secret key RWRGy8gxk9N934nSfLrJXlgJ9koqHc4Hdkuia2Pp6iARk9HXOTg4YlPa6nGllnDg11YsVDZxTDLCznTiDEwCjZEJNsM5wwqUEwk= OpenBSD 5.5 errata 21, February 20, 2015: Information leak in the XkbSetGeometry request of X servers Olivier Fourdan from Red Hat has discovered a protocol handling issue in the way the X server code base handles the XkbSetGeometry request. Apply patch using: signify -Vep /etc/signify/openbsd-55-base.pub -x 021_xserver.patch.sig \ -m - | (cd /usr/xenocara && patch -p0) Then build and install a new xserver: cd /usr/xenocara/xserver make -f Makefile.bsd-wrapper obj make -f Makefile.bsd-wrapper build Index: xserver/xkb/xkb.c diff -u xserver/xkb/xkb.c:1.9 xenocara/xserver/xkb/xkb.c:1.9.4.1 --- xserver/xkb/xkb.c:1.9 Fri Jun 7 19:28:56 2013 +++ xserver/xkb/xkb.c Thu Feb 12 08:28:44 2015 @@ -4958,26 +4958,29 @@ /***====================================================================***/ -static char * -_GetCountedString(char **wire_inout, Bool swap) +static Status +_GetCountedString(char **wire_inout, ClientPtr client, char **str) { - char *wire, *str; - CARD16 len, *plen; + char *wire, *next; + CARD16 len; wire = *wire_inout; - plen = (CARD16 *) wire; - if (swap) { - swaps(plen); - } - len = *plen; - str = malloc(len + 1); - if (str) { - memcpy(str, &wire[2], len); - str[len] = '\0'; + len = *(CARD16 *) wire; + if (client->swapped) { + swaps(&len); } - wire += XkbPaddedSize(len + 2); - *wire_inout = wire; - return str; + next = wire + XkbPaddedSize(len + 2); + /* Check we're still within the size of the request */ + if (client->req_len < + bytes_to_int32(next - (char *) client->requestBuffer)) + return BadValue; + *str = malloc(len + 1); + if (!*str) + return BadAlloc; + memcpy(*str, &wire[2], len); + *(*str + len) = '\0'; + *wire_inout = next; + return Success; } static Status @@ -4986,25 +4989,29 @@ { char *wire; xkbDoodadWireDesc *dWire; + xkbAnyDoodadWireDesc any; + xkbTextDoodadWireDesc text; XkbDoodadPtr doodad; + Status status; dWire = (xkbDoodadWireDesc *) (*wire_inout); + any = dWire->any; wire = (char *) &dWire[1]; if (client->swapped) { - swapl(&dWire->any.name); - swaps(&dWire->any.top); - swaps(&dWire->any.left); - swaps(&dWire->any.angle); + swapl(&any.name); + swaps(&any.top); + swaps(&any.left); + swaps(&any.angle); } CHK_ATOM_ONLY(dWire->any.name); - doodad = XkbAddGeomDoodad(geom, section, dWire->any.name); + doodad = XkbAddGeomDoodad(geom, section, any.name); if (!doodad) return BadAlloc; doodad->any.type = dWire->any.type; doodad->any.priority = dWire->any.priority; - doodad->any.top = dWire->any.top; - doodad->any.left = dWire->any.left; - doodad->any.angle = dWire->any.angle; + doodad->any.top = any.top; + doodad->any.left = any.left; + doodad->any.angle = any.angle; switch (doodad->any.type) { case XkbOutlineDoodad: case XkbSolidDoodad: @@ -5027,15 +5034,22 @@ dWire->text.colorNdx); return BadMatch; } + text = dWire->text; if (client->swapped) { - swaps(&dWire->text.width); - swaps(&dWire->text.height); + swaps(&text.width); + swaps(&text.height); } - doodad->text.width = dWire->text.width; - doodad->text.height = dWire->text.height; + doodad->text.width = text.width; + doodad->text.height = text.height; doodad->text.color_ndx = dWire->text.colorNdx; - doodad->text.text = _GetCountedString(&wire, client->swapped); - doodad->text.font = _GetCountedString(&wire, client->swapped); + status = _GetCountedString(&wire, client, &doodad->text.text); + if (status != Success) + return status; + status = _GetCountedString(&wire, client, &doodad->text.font); + if (status != Success) { + free (doodad->text.text); + return status; + } break; case XkbIndicatorDoodad: if (dWire->indicator.onColorNdx >= geom->num_colors) { @@ -5070,7 +5084,9 @@ } doodad->logo.color_ndx = dWire->logo.colorNdx; 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; default: client->errorValue = _XkbErrCode2(0x4F, dWire->any.type); @@ -5302,18 +5318,20 @@ char *wire; 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++) { char *name, *val; - name = _GetCountedString(&wire, client->swapped); - if (!name) - return BadAlloc; - val = _GetCountedString(&wire, client->swapped); - if (!val) { + status = _GetCountedString(&wire, client, &name); + if (status != Success) + return status; + status = _GetCountedString(&wire, client, &val); + if (status != Success) { free(name); - return BadAlloc; + return status; } if (XkbAddGeomProperty(geom, name, val) == NULL) { free(name); @@ -5347,9 +5365,9 @@ for (i = 0; i < req->nColors; i++) { char *name; - name = _GetCountedString(&wire, client->swapped); - if (!name) - return BadAlloc; + status = _GetCountedString(&wire, client, &name); + if (status != Success) + return status; if (!XkbAddGeomColor(geom, name, geom->num_colors)) { free(name); return BadAlloc;