Xi: unvalidated lengths in Xinput extension [CVE-2014-8095]

Multiple functions in the Xinput extension handling of requests from
clients failed to check that the length of the request sent by the
client was large enough to perform all the required operations and
thus could read or write to memory outside the bounds of the request
buffer.

This commit includes the creation of a new REQUEST_AT_LEAST_EXTRA_SIZE
macro in include/dix.h for the common case of needing to ensure a
request is large enough to include both the request itself and a
minimum amount of extra data following the request header.

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

Xi/chgdctl.c

@@ -78,7 +78,7 @@ SProcXChangeDeviceControl(ClientPtr client)
 
     REQUEST(xChangeDeviceControlReq);
     swaps(&stuff->length);
-    REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
+    REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
     swaps(&stuff->control);
     ctl = (xDeviceCtl *) &stuff[1];
     swaps(&ctl->control);
@@ -115,7 +115,7 @@ ProcXChangeDeviceControl(ClientPtr client)
     xDeviceEnableCtl *e;
 
     REQUEST(xChangeDeviceControlReq);
-    REQUEST_AT_LEAST_SIZE(xChangeDeviceControlReq);
+    REQUEST_AT_LEAST_EXTRA_SIZE(xChangeDeviceControlReq, sizeof(xDeviceCtl));
 
     len = stuff->length - bytes_to_int32(sizeof(xChangeDeviceControlReq));
     ret = dixLookupDevice(&dev, stuff->deviceid, client, DixManageAccess);
@@ -192,6 +192,10 @@ ProcXChangeDeviceControl(ClientPtr client)
         break;
     case DEVICE_ENABLE:
         e = (xDeviceEnableCtl *) &stuff[1];
+        if ((len != bytes_to_int32(sizeof(xDeviceEnableCtl)))) {
+            ret = BadLength;
+            goto out;
+        }
 
         if (IsXTestDevice(dev, NULL))
             status = !Success;

Xi/chgfctl.c

@@ -467,6 +467,8 @@ ProcXChangeFeedbackControl(ClientPtr client)
         xStringFeedbackCtl *f = ((xStringFeedbackCtl *) &stuff[1]);
 
         if (client->swapped) {
+            if (len < bytes_to_int32(sizeof(xStringFeedbackCtl)))
+                return BadLength;
             swaps(&f->num_keysyms);
         }
         if (len !=

Xi/sendexev.c

@@ -135,6 +135,9 @@ ProcXSendExtensionEvent(ClientPtr client)
     if (ret != Success)
         return ret;
 
+    if (stuff->num_events == 0)
+        return ret;
+
     /* The client's event type must be one defined by an extension. */
 
     first = ((xEvent *) &stuff[1]);

Xi/xiallowev.c

@@ -48,6 +48,7 @@ int
 SProcXIAllowEvents(ClientPtr client)
 {
     REQUEST(xXIAllowEventsReq);
+    REQUEST_AT_LEAST_SIZE(xXIAllowEventsReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
@@ -55,6 +56,7 @@ SProcXIAllowEvents(ClientPtr client)
     if (stuff->length > 3) {
         xXI2_2AllowEventsReq *req_xi22 = (xXI2_2AllowEventsReq *) stuff;
 
+        REQUEST_AT_LEAST_SIZE(xXI2_2AllowEventsReq);
         swapl(&req_xi22->touchid);
         swapl(&req_xi22->grab_window);
     }

Xi/xichangecursor.c

@@ -57,11 +57,11 @@ int
 SProcXIChangeCursor(ClientPtr client)
 {
     REQUEST(xXIChangeCursorReq);
+    REQUEST_SIZE_MATCH(xXIChangeCursorReq);
     swaps(&stuff->length);
     swapl(&stuff->win);
     swapl(&stuff->cursor);
     swaps(&stuff->deviceid);
-    REQUEST_SIZE_MATCH(xXIChangeCursorReq);
     return (ProcXIChangeCursor(client));
 }
 

Xi/xichangehierarchy.c

@@ -411,7 +411,7 @@ int
 ProcXIChangeHierarchy(ClientPtr client)
 {
     xXIAnyHierarchyChangeInfo *any;
-    int required_len = sizeof(xXIChangeHierarchyReq);
+    size_t len;			/* length of data remaining in request */
     int rc = Success;
     int flags[MAXDEVICES] = { 0 };
 
@@ -421,21 +421,46 @@ ProcXIChangeHierarchy(ClientPtr client)
     if (!stuff->num_changes)
         return rc;
 
+    if (stuff->length > (INT_MAX >> 2))
+        return BadAlloc;
+    len = (stuff->length << 2) - sizeof(xXIAnyHierarchyChangeInfo);
+
     any = (xXIAnyHierarchyChangeInfo *) &stuff[1];
     while (stuff->num_changes--) {
+        if (len < sizeof(xXIAnyHierarchyChangeInfo)) {
+            rc = BadLength;
+            goto unwind;
+        }
+
         SWAPIF(swaps(&any->type));
         SWAPIF(swaps(&any->length));
 
-        required_len += any->length;
-        if ((stuff->length * 4) < required_len)
+        if ((any->length > (INT_MAX >> 2)) || (len < (any->length << 2)))
             return BadLength;
 
+#define CHANGE_SIZE_MATCH(type) \
+    do { \
+        if ((len < sizeof(type)) || (any->length != (sizeof(type) >> 2))) { \
+            rc = BadLength; \
+            goto unwind; \
+        } \
+    } while(0)
+
         switch (any->type) {
         case XIAddMaster:
         {
             xXIAddMasterInfo *c = (xXIAddMasterInfo *) any;
 
+            /* Variable length, due to appended name string */
+            if (len < sizeof(xXIAddMasterInfo)) {
+                rc = BadLength;
+                goto unwind;
+            }
             SWAPIF(swaps(&c->name_len));
+            if (c->name_len > (len - sizeof(xXIAddMasterInfo))) {
+                rc = BadLength;
+                goto unwind;
+            }
 
             rc = add_master(client, c, flags);
             if (rc != Success)
@@ -446,6 +471,7 @@ ProcXIChangeHierarchy(ClientPtr client)
         {
             xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;
 
+            CHANGE_SIZE_MATCH(xXIRemoveMasterInfo);
             rc = remove_master(client, r, flags);
             if (rc != Success)
                 goto unwind;
@@ -455,6 +481,7 @@ ProcXIChangeHierarchy(ClientPtr client)
         {
             xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;
 
+            CHANGE_SIZE_MATCH(xXIDetachSlaveInfo);
             rc = detach_slave(client, c, flags);
             if (rc != Success)
                 goto unwind;
@@ -464,6 +491,7 @@ ProcXIChangeHierarchy(ClientPtr client)
         {
             xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;
 
+            CHANGE_SIZE_MATCH(xXIAttachSlaveInfo);
             rc = attach_slave(client, c, flags);
             if (rc != Success)
                 goto unwind;
@@ -471,6 +499,7 @@ ProcXIChangeHierarchy(ClientPtr client)
             break;
         }
 
+        len -= any->length * 4;
         any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);
     }
 

Xi/xigetclientpointer.c

@@ -50,6 +50,7 @@ int
 SProcXIGetClientPointer(ClientPtr client)
 {
     REQUEST(xXIGetClientPointerReq);
+    REQUEST_SIZE_MATCH(xXIGetClientPointerReq);
 
     swaps(&stuff->length);
     swapl(&stuff->win);

Xi/xigrabdev.c

@@ -47,6 +47,11 @@ int
 SProcXIGrabDevice(ClientPtr client)
 {
     REQUEST(xXIGrabDeviceReq);
+    /*
+     * Check here for at least the length of the struct we swap, then
+     * let ProcXIGrabDevice check the full size after we swap mask_len.
+     */
+    REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
@@ -71,7 +76,7 @@ ProcXIGrabDevice(ClientPtr client)
     unsigned int pointer_mode;
 
     REQUEST(xXIGrabDeviceReq);
-    REQUEST_AT_LEAST_SIZE(xXIGrabDeviceReq);
+    REQUEST_FIXED_SIZE(xXIGrabDeviceReq, ((size_t) stuff->mask_len) * 4);
 
     ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGrabAccess);
     if (ret != Success)
@@ -131,6 +136,7 @@ int
 SProcXIUngrabDevice(ClientPtr client)
 {
     REQUEST(xXIUngrabDeviceReq);
+    REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
@@ -148,6 +154,7 @@ ProcXIUngrabDevice(ClientPtr client)
     TimeStamp time;
 
     REQUEST(xXIUngrabDeviceReq);
+    REQUEST_SIZE_MATCH(xXIUngrabDeviceReq);
 
     ret = dixLookupDevice(&dev, stuff->deviceid, client, DixGetAttrAccess);
     if (ret != Success)

Xi/xipassivegrab.c

@@ -53,6 +53,7 @@ SProcXIPassiveGrabDevice(ClientPtr client)
     uint32_t *mods;
 
     REQUEST(xXIPassiveGrabDeviceReq);
+    REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
@@ -63,6 +64,8 @@ SProcXIPassiveGrabDevice(ClientPtr client)
     swaps(&stuff->mask_len);
     swaps(&stuff->num_modifiers);
 
+    REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
+        ((uint32_t) stuff->mask_len + stuff->num_modifiers) *4);
     mods = (uint32_t *) &stuff[1] + stuff->mask_len;
 
     for (i = 0; i < stuff->num_modifiers; i++, mods++) {
@@ -92,7 +95,8 @@ ProcXIPassiveGrabDevice(ClientPtr client)
     int mask_len;
 
     REQUEST(xXIPassiveGrabDeviceReq);
-    REQUEST_AT_LEAST_SIZE(xXIPassiveGrabDeviceReq);
+    REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
+        ((uint32_t) stuff->mask_len + stuff->num_modifiers) * 4);
 
     if (stuff->deviceid == XIAllDevices)
         dev = inputInfo.all_devices;
@@ -252,6 +256,7 @@ SProcXIPassiveUngrabDevice(ClientPtr client)
     uint32_t *modifiers;
 
     REQUEST(xXIPassiveUngrabDeviceReq);
+    REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
 
     swaps(&stuff->length);
     swapl(&stuff->grab_window);
@@ -259,6 +264,8 @@ SProcXIPassiveUngrabDevice(ClientPtr client)
     swapl(&stuff->detail);
     swaps(&stuff->num_modifiers);
 
+    REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
+                       ((uint32_t) stuff->num_modifiers) << 2);
     modifiers = (uint32_t *) &stuff[1];
 
     for (i = 0; i < stuff->num_modifiers; i++, modifiers++)
@@ -277,7 +284,8 @@ ProcXIPassiveUngrabDevice(ClientPtr client)
     int i, rc;
 
     REQUEST(xXIPassiveUngrabDeviceReq);
-    REQUEST_AT_LEAST_SIZE(xXIPassiveUngrabDeviceReq);
+    REQUEST_FIXED_SIZE(xXIPassiveUngrabDeviceReq,
+                       ((uint32_t) stuff->num_modifiers) << 2);
 
     if (stuff->deviceid == XIAllDevices)
         dev = inputInfo.all_devices;

Xi/xiproperty.c

@@ -1013,10 +1013,9 @@ int
 SProcXListDeviceProperties(ClientPtr client)
 {
     REQUEST(xListDevicePropertiesReq);
+    REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
 
     swaps(&stuff->length);
-
-    REQUEST_SIZE_MATCH(xListDevicePropertiesReq);
     return (ProcXListDeviceProperties(client));
 }
 
@@ -1037,10 +1036,10 @@ int
 SProcXDeleteDeviceProperty(ClientPtr client)
 {
     REQUEST(xDeleteDevicePropertyReq);
+    REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
 
     swaps(&stuff->length);
     swapl(&stuff->property);
-    REQUEST_SIZE_MATCH(xDeleteDevicePropertyReq);
     return (ProcXDeleteDeviceProperty(client));
 }
 
@@ -1048,13 +1047,13 @@ int
 SProcXGetDeviceProperty(ClientPtr client)
 {
     REQUEST(xGetDevicePropertyReq);
+    REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
 
     swaps(&stuff->length);
     swapl(&stuff->property);
     swapl(&stuff->type);
     swapl(&stuff->longOffset);
     swapl(&stuff->longLength);
-    REQUEST_SIZE_MATCH(xGetDevicePropertyReq);
     return (ProcXGetDeviceProperty(client));
 }
 
@@ -1253,11 +1252,10 @@ int
 SProcXIListProperties(ClientPtr client)
 {
     REQUEST(xXIListPropertiesReq);
+    REQUEST_SIZE_MATCH(xXIListPropertiesReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
-
-    REQUEST_SIZE_MATCH(xXIListPropertiesReq);
     return (ProcXIListProperties(client));
 }
 
@@ -1279,11 +1277,11 @@ int
 SProcXIDeleteProperty(ClientPtr client)
 {
     REQUEST(xXIDeletePropertyReq);
+    REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
     swapl(&stuff->property);
-    REQUEST_SIZE_MATCH(xXIDeletePropertyReq);
     return (ProcXIDeleteProperty(client));
 }
 
@@ -1291,6 +1289,7 @@ int
 SProcXIGetProperty(ClientPtr client)
 {
     REQUEST(xXIGetPropertyReq);
+    REQUEST_SIZE_MATCH(xXIGetPropertyReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
@@ -1298,7 +1297,6 @@ SProcXIGetProperty(ClientPtr client)
     swapl(&stuff->type);
     swapl(&stuff->offset);
     swapl(&stuff->len);
-    REQUEST_SIZE_MATCH(xXIGetPropertyReq);
     return (ProcXIGetProperty(client));
 }
 

Xi/xiquerydevice.c

@@ -54,6 +54,7 @@ int
 SProcXIQueryDevice(ClientPtr client)
 {
     REQUEST(xXIQueryDeviceReq);
+    REQUEST_SIZE_MATCH(xXIQueryDeviceReq);
 
     swaps(&stuff->length);
     swaps(&stuff->deviceid);

Xi/xiquerypointer.c

@@ -63,6 +63,8 @@ int
 SProcXIQueryPointer(ClientPtr client)
 {
     REQUEST(xXIQueryPointerReq);
+    REQUEST_SIZE_MATCH(xXIQueryPointerReq);
+
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
     swapl(&stuff->win);

Xi/xiselectev.c

@@ -114,6 +114,7 @@ int
 SProcXISelectEvents(ClientPtr client)
 {
     int i;
+    int len;
     xXIEventMask *evmask;
 
     REQUEST(xXISelectEventsReq);
@@ -122,10 +123,17 @@ SProcXISelectEvents(ClientPtr client)
     swapl(&stuff->win);
     swaps(&stuff->num_masks);
 
+    len = stuff->length - bytes_to_int32(sizeof(xXISelectEventsReq));
     evmask = (xXIEventMask *) &stuff[1];
     for (i = 0; i < stuff->num_masks; i++) {
+        if (len < bytes_to_int32(sizeof(xXIEventMask)))
+            return BadLength;
+        len -= bytes_to_int32(sizeof(xXIEventMask));
         swaps(&evmask->deviceid);
         swaps(&evmask->mask_len);
+        if (len < evmask->mask_len)
+            return BadLength;
+        len -= evmask->mask_len;
         evmask =
             (xXIEventMask *) (((char *) &evmask[1]) + evmask->mask_len * 4);
     }

Xi/xisetclientpointer.c

@@ -51,10 +51,11 @@ int
 SProcXISetClientPointer(ClientPtr client)
 {
     REQUEST(xXISetClientPointerReq);
+    REQUEST_SIZE_MATCH(xXISetClientPointerReq);
+
     swaps(&stuff->length);
     swapl(&stuff->win);
     swaps(&stuff->deviceid);
-    REQUEST_SIZE_MATCH(xXISetClientPointerReq);
     return (ProcXISetClientPointer(client));
 }
 

Xi/xisetdevfocus.c

@@ -44,6 +44,8 @@ int
 SProcXISetFocus(ClientPtr client)
 {
     REQUEST(xXISetFocusReq);
+    REQUEST_AT_LEAST_SIZE(xXISetFocusReq);
+
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
     swapl(&stuff->focus);
@@ -56,6 +58,8 @@ int
 SProcXIGetFocus(ClientPtr client)
 {
     REQUEST(xXIGetFocusReq);
+    REQUEST_AT_LEAST_SIZE(xXIGetFocusReq);
+
     swaps(&stuff->length);
     swaps(&stuff->deviceid);
 

Xi/xiwarppointer.c

@@ -56,6 +56,8 @@ int
 SProcXIWarpPointer(ClientPtr client)
 {
     REQUEST(xXIWarpPointerReq);
+    REQUEST_SIZE_MATCH(xXIWarpPointerReq);
+
     swaps(&stuff->length);
     swapl(&stuff->src_win);
     swapl(&stuff->dst_win);

include/dix.h

@@ -74,6 +74,10 @@ SOFTWARE.
     if ((sizeof(req) >> 2) > client->req_len )\
          return(BadLength)
 
+#define REQUEST_AT_LEAST_EXTRA_SIZE(req, extra)  \
+    if (((sizeof(req) + ((uint64_t) extra)) >> 2) > client->req_len ) \
+         return(BadLength)
+
 #define REQUEST_FIXED_SIZE(req, n)\
     if (((sizeof(req) >> 2) > client->req_len) || \
         ((n >> 2) >= client->req_len) || \