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 <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 20079c36cf7d377938ca5478447d8b9045cb7d43)
(cherry picked from commit f160e722672dbb2b5215870b47bcc51461d96ff1)
Signed-off-by: Julien Cristau <jcristau@debian.org>

1 files 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;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) {
             xfree(name);
-            return BadAlloc;
+            return status;
         }
 	if (XkbAddGeomProperty(geom,name,val)==NULL) {
             xfree(name);
@@ -4833,9 +4848,10 @@ char *		wire;
 
     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)) {
             xfree(name);
 	    return BadAlloc;