From ab0fd32fb12b2153177dd101976c9dd23793b947 Mon Sep 17 00:00:00 2001

From: Olivier Fourdan <ofourdan@redhat.com>

Date: Fri, 16 Jan 2015 08:44:45 +0100

Subject: [PATCH] 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>

---

 xkb/xkb.c | 65 +++++++++++++++++++++++++++++++++++++++------------------------

 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/xkb/xkb.c b/xkb/xkb.c

index b9a3ac4..f3988f9 100644

--- a/xkb/xkb.c

+++ b/xkb/xkb.c

@@ -4957,25 +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;

+    char *wire, *next;

     CARD16 len;

     wire = *wire_inout;

     len = *(CARD16 *) wire;

-    if (swap) {

+    if (client->swapped) {

         swaps(&len;;

     }

-    str = malloc(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

@@ -4987,6 +4991,7 @@ _CheckSetDoodad(char **wire_inout,

     xkbAnyDoodadWireDesc any;

     xkbTextDoodadWireDesc text;

     XkbDoodadPtr doodad;

+    Status status;

     dWire = (xkbDoodadWireDesc *) (*wire_inout);

     any = dWire->any;

@@ -5036,8 +5041,14 @@ _CheckSetDoodad(char **wire_inout,

         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) {

@@ -5072,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);

@@ -5304,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);

@@ -5349,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;

-- 

2.1.0