1 files changed, 59 insertions, 41 deletions

diff --git a/xorg-server/xkb/xkb.c b/xorg-server/xkb/xkb.c
index 15c7f34cf..f3988f9a7 100644
--- a/xorg-server/xkb/xkb.c
+++ b/xorg-server/xkb/xkb.c
@@ -4957,26 +4957,29 @@ ProcXkbGetGeometry(ClientPtr client)
 
 /***====================================================================***/
 
-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
@@ -4985,25 +4988,29 @@ _CheckSetDoodad(char **wire_inout,
 {
     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:
@@ -5026,15 +5033,22 @@ _CheckSetDoodad(char **wire_inout,
                                               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) {
@@ -5069,7 +5083,9 @@ _CheckSetDoodad(char **wire_inout,
         }
         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);
@@ -5301,18 +5317,20 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
     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);
@@ -5346,9 +5364,9 @@ _CheckSetGeom(XkbGeometryPtr geom, xkbSetGeometryReq * req, ClientPtr client)
     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;