From 4c8e9bcab459ea5f870d3e56eff15f931807f9b7 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Tue, 28 May 2013 15:52:32 +1000
Subject: Fix potential corruption in mask_len handling

First: check for allocation failure on the mask.
XI2 requires that the mask is zeroed, so we can't just Data() the mask
provided by the client (it will pad) - we need a tmp buffer. Make sure that
doesn't fail.

Second:
req->mask_len is a uint16_t, so check against malicious mask_lens that would
cause us to corrupt memory on copy, as the code always allocates
req->mask_len * 4, but copies mask->mask_len bytes.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 src/XIGrabDevice.c  | 18 ++++++++++++------
 src/XIPassiveGrab.c |  9 ++++++++-
 src/XISelEv.c       | 30 +++++++++++++++++++++++++-----
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/src/XIGrabDevice.c b/src/XIGrabDevice.c
index dd1bd10..2bff3d8 100644
--- a/src/XIGrabDevice.c
+++ b/src/XIGrabDevice.c
@@ -50,6 +50,17 @@ XIGrabDevice(Display* dpy, int deviceid, Window grab_window, Time time,
     if (_XiCheckExtInit(dpy, XInput_2_0, extinfo) == -1)
 	return (NoSuchExtension);
 
+    if (mask->mask_len > INT_MAX - 3 ||
+        (mask->mask_len + 3)/4 >= 0xffff)
+        return BadValue;
+
+    /* mask->mask_len is in bytes, but we need 4-byte units on the wire,
+     * and they need to be padded with 0 */
+    len = (mask->mask_len + 3)/4;
+    buff = calloc(4, len);
+    if (!buff)
+        return BadAlloc;
+
     GetReq(XIGrabDevice, req);
     req->reqType  = extinfo->codes->major_opcode;
     req->ReqType  = X_XIGrabDevice;
@@ -59,14 +70,9 @@ XIGrabDevice(Display* dpy, int deviceid, Window grab_window, Time time,
     req->grab_mode = grab_mode;
     req->paired_device_mode = paired_device_mode;
     req->owner_events = owner_events;
-    req->mask_len = (mask->mask_len + 3)/4;
+    req->mask_len = len;
     req->cursor = cursor;
 
-
-    /* mask->mask_len is in bytes, but we need 4-byte units on the wire,
-     * and they need to be padded with 0 */
-    len = req->mask_len;
-    buff = calloc(1, len * 4);
     memcpy(buff, mask->mask, mask->mask_len);
 
     SetReqLen(req, len, len);
diff --git a/src/XIPassiveGrab.c b/src/XIPassiveGrab.c
index 53b4084..4ed2f09 100644
--- a/src/XIPassiveGrab.c
+++ b/src/XIPassiveGrab.c
@@ -51,6 +51,14 @@ _XIPassiveGrabDevice(Display* dpy, int deviceid, int grabtype, int detail,
     if (_XiCheckExtInit(dpy, XInput_2_0, extinfo) == -1)
 	return -1;
 
+    if (mask->mask_len > INT_MAX - 3 ||
+        (mask->mask_len + 3)/4 >= 0xffff)
+        return -1;
+
+    buff = calloc(4, (mask->mask_len + 3)/4);
+    if (!buff)
+        return -1;
+
     GetReq(XIPassiveGrabDevice, req);
     req->reqType = extinfo->codes->major_opcode;
     req->ReqType = X_XIPassiveGrabDevice;
@@ -68,7 +76,6 @@ _XIPassiveGrabDevice(Display* dpy, int deviceid, int grabtype, int detail,
     len = req->mask_len + num_modifiers;
     SetReqLen(req, len, len);
 
-    buff = calloc(4, req->mask_len);
     memcpy(buff, mask->mask, mask->mask_len);
     Data(dpy, buff, req->mask_len * 4);
     for (i = 0; i < num_modifiers; i++)
diff --git a/src/XISelEv.c b/src/XISelEv.c
index 0471bef..55c0a6a 100644
--- a/src/XISelEv.c
+++ b/src/XISelEv.c
@@ -53,6 +53,8 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* masks, int num_masks)
     int i;
     int len = 0;
     int r = Success;
+    int max_mask_len = 0;
+    char *buff;
 
     XExtDisplayInfo *info = XInput_find_display(dpy);
     LockDisplay(dpy);
@@ -60,6 +62,26 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* masks, int num_masks)
         r = NoSuchExtension;
         goto out;
     }
+
+    for (i = 0; i < num_masks; i++) {
+        current = &masks[i];
+        if (current->mask_len > INT_MAX - 3 ||
+            (current->mask_len + 3)/4 >= 0xffff) {
+            r = -1;
+            goto out;
+        }
+        if (current->mask_len > max_mask_len)
+            max_mask_len = current->mask_len;
+    }
+
+    /* max_mask_len is in bytes, but we need 4-byte units on the wire,
+     * and they need to be padded with 0 */
+    buff = calloc(4, ((max_mask_len + 3)/4));
+    if (!buff) {
+        r = -1;
+        goto out;
+    }
+
     GetReq(XISelectEvents, req);
 
     req->reqType = info->codes->major_opcode;
@@ -79,19 +101,17 @@ XISelectEvents(Display* dpy, Window win, XIEventMask* masks, int num_masks)
 
     for (i = 0; i < num_masks; i++)
     {
-        char *buff;
         current = &masks[i];
         mask.deviceid = current->deviceid;
         mask.mask_len = (current->mask_len + 3)/4;
-        /* masks.mask_len is in bytes, but we need 4-byte units on the wire,
-         * and they need to be padded with 0 */
-        buff = calloc(1, mask.mask_len * 4);
+
+        memset(buff, 0, max_mask_len);
         memcpy(buff, current->mask, current->mask_len);
         Data(dpy, (char*)&mask, sizeof(xXIEventMask));
         Data(dpy, buff, mask.mask_len * 4);
-        free(buff);
     }
 
+    free(buff);
 out:
     UnlockDisplay(dpy);
     SyncHandle();
-- 
cgit v1.2.3