From d7258444a876a65986212c10ddcaa1783af558bf Mon Sep 17 00:00:00 2001 From: Olivier Fourdan Date: Fri, 16 Jan 2015 08:44:45 +0100 Subject: 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. v3: backport to nx-libs 3.6.x because this is the CVE-2015-0255 fix (Mike DePaulo) Signed-off-by: Olivier Fourdan Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer (cherry picked from commit 20079c36cf7d377938ca5478447d8b9045cb7d43) (cherry picked from commit f160e722672dbb2b5215870b47bcc51461d96ff1) Signed-off-by: Julien Cristau --- nx-X11/programs/Xserver/xkb/xkb.c | 66 ++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/nx-X11/programs/Xserver/xkb/xkb.c b/nx-X11/programs/Xserver/xkb/xkb.c index d8b5b2ce1..778269f7c 100644 --- a/nx-X11/programs/Xserver/xkb/xkb.c +++ b/nx-X11/programs/Xserver/xkb/xkb.c @@ -4437,26 +4437,30 @@ ProcXkbGetGeometry(ClientPtr client) /***====================================================================***/ -static char * -_GetCountedString(char **wire_inout,Bool swap) +static Status +_GetCountedString(char **wire_inout, ClientPtr client, char **str) { -char * wire,*str; +char * wire, *next; CARD16 len; wire= *wire_inout; len= (CARD16 *)wire; - if (swap) { + if (client->swapped) { register int n; swaps(&len, n); } - str= (char *)_XkbAlloc(len+1); - if (str) { - memcpy(str,&wire[2],len); - str[len]= '\0'; - } - 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 @@ -4470,6 +4474,7 @@ xkbDoodadWireDesc * dWire; xkbAnyDoodadWireDesc any; xkbTextDoodadWireDesc text; XkbDoodadPtr doodad; +Status status; dWire= (xkbDoodadWireDesc *)(*wire_inout); any = dWire->any; @@ -4521,8 +4526,14 @@ XkbDoodadPtr doodad; 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) { @@ -4557,7 +4568,9 @@ XkbDoodadPtr doodad; } 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); @@ -4792,17 +4805,19 @@ Status status; 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;inProperties;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) { xfree(name); - return BadAlloc; + return status; } if (XkbAddGeomProperty(geom,name,val)==NULL) { xfree(name); @@ -4833,9 +4848,10 @@ char * wire; for (i=0;inColors;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)) { xfree(name); return BadAlloc; -- cgit v1.2.3