[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

libxi: Changes to 'upstream-unstable'



 autogen.sh                       |    4 +
 configure.ac                     |   13 +++-
 include/X11/extensions/XInput2.h |   47 ++++++++++++++++
 man/Makefile.am                  |    9 ++-
 man/XGetDeviceControl.txt        |   12 ++--
 man/XIBarrierReleasePointer.txt  |   76 ++++++++++++++++++++++++++
 man/XIGrabButton.txt             |    3 -
 src/Makefile.am                  |    4 +
 src/XExtInt.c                    |  110 +++++++++++++++++++++++++++------------
 src/XGMotion.c                   |   24 ++++++--
 src/XGetBMap.c                   |   21 ++++---
 src/XGetDCtl.c                   |   41 ++++++++++----
 src/XGetDProp.c                  |   64 +++++++++++++---------
 src/XGetFCtl.c                   |   40 +++++++++-----
 src/XGetKMap.c                   |    2 
 src/XGetMMap.c                   |    2 
 src/XGetProp.c                   |   16 +++--
 src/XGtSelect.c                  |    2 
 src/XIBarrier.c                  |   81 ++++++++++++++++++++++++++++
 src/XIGrabDevice.c               |   19 ++++--
 src/XIPassiveGrab.c              |   12 +++-
 src/XIProperties.c               |   18 +++---
 src/XIQueryVersion.c             |    6 +-
 src/XISelEv.c                    |   65 ++++++++++++++++++-----
 src/XIint.h                      |   15 +++++
 src/XListDProp.c                 |    2 
 src/XListDev.c                   |   31 ++++++----
 src/XOpenDev.c                   |    2 
 src/XQueryDv.c                   |   19 ++++--
 xi.pc.in                         |    2 
 30 files changed, 589 insertions(+), 173 deletions(-)

New commits:
commit 957a9d64afd76f878ce6c5570f369e2a7fc1e772
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Jun 27 08:47:16 2013 +1000

    libXi 1.7.1.901
    
    Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/configure.ac b/configure.ac
index f5ef1e2..18d895b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,7 +1,7 @@
 
 # Initialize Autoconf
 AC_PREREQ([2.60])
-AC_INIT([libXi], [1.7.1],
+AC_INIT([libXi], [1.7.1.901],
 	[https://bugs.freedesktop.org/enter_bug.cgi?product=xorg], [libXi])
 AC_CONFIG_SRCDIR([Makefile.am])
 AC_CONFIG_HEADERS([src/config.h])

commit 62033a9c83bcdc75b9f1452ce24729eefa8f4dc0
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Jun 27 06:25:02 2013 +1000

    Include limits.h to prevent build error: missing INT_MAX
    
    Introduced in 4c8e9bcab459ea5f870d3e56eff15f931807f9b7.
    
    Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XIGrabDevice.c b/src/XIGrabDevice.c
index 2bff3d8..a8c5697 100644
--- a/src/XIGrabDevice.c
+++ b/src/XIGrabDevice.c
@@ -31,6 +31,7 @@
 #include <X11/extensions/XI2proto.h>
 #include <X11/extensions/XInput2.h>
 #include <X11/extensions/extutil.h>
+#include <limits.h>
 #include "XIint.h"
 
 
diff --git a/src/XIPassiveGrab.c b/src/XIPassiveGrab.c
index 4ed2f09..baadccb 100644
--- a/src/XIPassiveGrab.c
+++ b/src/XIPassiveGrab.c
@@ -30,6 +30,7 @@
 #include <X11/extensions/XI2proto.h>
 #include <X11/extensions/XInput2.h>
 #include <X11/extensions/extutil.h>
+#include <limits.h>
 #include "XIint.h"
 
 static int

commit 0f3f5a36d5fc6dc53f69f48a0c83aef6a1fcf381
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue May 28 15:52:34 2013 +1000

    If the XGetDeviceDontPropagateList reply has an invalid length, return 0
    
    If we skip over the reply data, return 0 as number of event classes.
    
    Follow-up to 6dd6dc51a2935c72774be81e5cc2ba2c30e9feff.
    
    Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetProp.c b/src/XGetProp.c
index b49328c..8c69ef2 100644
--- a/src/XGetProp.c
+++ b/src/XGetProp.c
@@ -104,8 +104,10 @@ XGetDeviceDontPropagateList(
 		_XRead(dpy, (char *)(&ec), sizeof(CARD32));
 		list[i] = (XEventClass) ec;
 	    }
-	} else
+	} else {
+            *count = 0;
 	    _XEatDataWords(dpy, rep.length);
+        }
     }
 
     UnlockDisplay(dpy);

commit 35ae16dc2f16b24a22625b2d9f76a2128b673a6c
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue May 28 15:52:33 2013 +1000

    Change size += to size = in XGetDeviceControl
    
    size += blah is technically correct but it implies that we're looping or
    otherwise incrementing the size. Which we don't, it's only ever set once.
    
    Change this to avoid reviewer confusion.
    
    Reported-by: Dave "color-me-confused" Airlie <airlied@redhat.com>
    Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetDCtl.c b/src/XGetDCtl.c
index 51ed0ae..b576aa5 100644
--- a/src/XGetDCtl.c
+++ b/src/XGetDCtl.c
@@ -122,34 +122,34 @@ XGetDeviceControl(
 	    val_size = 3 * sizeof(int) * r->num_valuators;
 	    if ((sizeof(xDeviceResolutionState) + val_size) > nbytes)
 		goto out;
-	    size += sizeof(XDeviceResolutionState) + val_size;
+	    size = sizeof(XDeviceResolutionState) + val_size;
 	    break;
 	}
         case DEVICE_ABS_CALIB:
         {
             if (sizeof(xDeviceAbsCalibState) > nbytes)
                 goto out;
-            size += sizeof(XDeviceAbsCalibState);
+            size = sizeof(XDeviceAbsCalibState);
             break;
         }
         case DEVICE_ABS_AREA:
         {
             if (sizeof(xDeviceAbsAreaState) > nbytes)
                 goto out;
-            size += sizeof(XDeviceAbsAreaState);
+            size = sizeof(XDeviceAbsAreaState);
             break;
         }
         case DEVICE_CORE:
         {
             if (sizeof(xDeviceCoreState) > nbytes)
                 goto out;
-            size += sizeof(XDeviceCoreState);
+            size = sizeof(XDeviceCoreState);
             break;
         }
 	default:
 	    if (d->length > nbytes)
 		goto out;
-	    size += d->length;
+	    size = d->length;
 	    break;
 	}
 

commit 4c8e9bcab459ea5f870d3e56eff15f931807f9b7
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue May 28 15:52:32 2013 +1000

    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>

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();

commit 661c45ca17c434dbd342a46fd3fb813852ae0ca9
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue May 21 12:23:05 2013 +1000

    Don't overwrite the cookies serial number
    
    serial != sequenceNumber, see _XSetLastRequestRead()
    
    cookie->serial is already set at this point, setting it again directly from
    the sequenceNumber of the event causes a bunch of weird issues such as
    scrollbars and text drag-n-drop breaking.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=965347
    
    Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XExtInt.c b/src/XExtInt.c
index 8e19b97..d3c6b7c 100644
--- a/src/XExtInt.c
+++ b/src/XExtInt.c
@@ -915,7 +915,6 @@ static void xge_copy_to_cookie(xGenericEvent* ev,
     cookie->type = ev->type;
     cookie->evtype = ev->evtype;
     cookie->extension = ev->extension;
-    cookie->serial = ev->sequenceNumber;
 }
 
 static Bool

commit 81b4df8ac6aa1520c41c3526961014a6f115cc46
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sun Mar 10 00:16:22 2013 -0800

    sign extension issue in XListInputDevices() [CVE-2013-1995]
    
    nptr is (signed) char, which can be negative, and will sign extend
    when added to the int size, which means size can be subtracted from,
    leading to allocating too small a buffer to hold the data being copied
    from the X server's reply.
    
    v2: check that string size fits inside the data read from the server,
        so that we don't read out of bounds either
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XListDev.c b/src/XListDev.c
index 1c14b96..b85ff3c 100644
--- a/src/XListDev.c
+++ b/src/XListDev.c
@@ -73,7 +73,7 @@ static int pad_to_xid(int base_size)
     return ((base_size + padsize - 1)/padsize) * padsize;
 }
 
-static int
+static size_t
 SizeClassInfo(xAnyClassPtr *any, int num_classes)
 {
     int size = 0;
@@ -170,7 +170,7 @@ XListInputDevices(
     register Display	*dpy,
     int			*ndevices)
 {
-    int size;
+    size_t size;
     xListInputDevicesReq *req;
     xListInputDevicesReply rep;
     xDeviceInfo *list, *slist = NULL;
@@ -178,7 +178,7 @@ XListInputDevices(
     XDeviceInfo *clist = NULL;
     xAnyClassPtr any, sav_any;
     XAnyClassPtr Any;
-    char *nptr, *Nptr;
+    unsigned char *nptr, *Nptr;
     int i;
     unsigned long rlen;
     XExtDisplayInfo *info = XInput_find_display(dpy);
@@ -217,9 +217,12 @@ XListInputDevices(
             size += SizeClassInfo(&any, (int)list->num_classes);
 	}
 
-	for (i = 0, nptr = (char *)any; i < *ndevices; i++) {
+	Nptr = ((unsigned char *)list) + rlen + 1;
+	for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) {
 	    size += *nptr + 1;
 	    nptr += (*nptr + 1);
+	    if (nptr > Nptr)
+		goto out;
 	}
 
 	clist = (XDeviceInfoPtr) Xmalloc(size);
@@ -245,8 +248,8 @@ XListInputDevices(
 	}
 
 	clist = sclist;
-	nptr = (char *)any;
-	Nptr = (char *)Any;
+	nptr = (unsigned char *)any;
+	Nptr = (unsigned char *)Any;
 	for (i = 0; i < *ndevices; i++, clist++) {
 	    clist->name = (char *)Nptr;
 	    memcpy(Nptr, nptr + 1, *nptr);
@@ -256,6 +259,7 @@ XListInputDevices(
 	}
     }
 
+  out:
     XFree((char *)slist);
     UnlockDisplay(dpy);
     SyncHandle();

commit ef82512288d8ca36ac0beeb289f158195b0a8cae
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sun Mar 10 00:22:14 2013 -0800

    Avoid integer overflow in XListInputDevices() [CVE-2013-1984 8/8]
    
    If the length of the reply as reported by the Xserver is too long, it
    could overflow the calculation for the size of the buffer to copy the
    reply into, causing memory corruption.
    
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XListDev.c b/src/XListDev.c
index 1fa4747..1c14b96 100644
--- a/src/XListDev.c
+++ b/src/XListDev.c
@@ -60,6 +60,7 @@ SOFTWARE.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 /* Calculate length field to a multiples of sizeof(XID). XIDs are typedefs
  * to ulong and thus may be 8 bytes on some platforms. This can trigger a
@@ -179,7 +180,7 @@ XListInputDevices(
     XAnyClassPtr Any;
     char *nptr, *Nptr;
     int i;
-    long rlen;
+    unsigned long rlen;
     XExtDisplayInfo *info = XInput_find_display(dpy);
 
     LockDisplay(dpy);
@@ -198,9 +199,10 @@ XListInputDevices(
 
     if ((*ndevices = rep.ndevices)) {	/* at least 1 input device */
 	size = *ndevices * sizeof(XDeviceInfo);
-	rlen = rep.length << 2;	/* multiply length by 4    */
-	list = (xDeviceInfo *) Xmalloc(rlen);
-	slist = list;
+	if (rep.length < (INT_MAX >> 2)) {
+	    rlen = rep.length << 2;	/* multiply length by 4    */
+	    slist = list = Xmalloc(rlen);
+	}
 	if (!slist) {
 	    _XEatDataWords(dpy, rep.length);
 	    UnlockDisplay(dpy);

commit 17071c1c608247800b2ca03a35b1fcc9c4cabe6c
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sun Mar 10 13:30:55 2013 -0700

    Avoid integer overflow in XGetDeviceProperties() [CVE-2013-1984 7/8]
    
    If the number of items as reported by the Xserver is too large, it
    could overflow the calculation for the size of the buffer to copy the
    reply into, causing memory corruption.
    
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetDProp.c b/src/XGetDProp.c
index f9e8f0c..3691122 100644
--- a/src/XGetDProp.c
+++ b/src/XGetDProp.c
@@ -38,6 +38,7 @@ in this Software without prior written authorization from the author.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 int
 XGetDeviceProperty(Display* dpy, XDevice* dev,
@@ -48,7 +49,8 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
 {
     xGetDevicePropertyReq   *req;
     xGetDevicePropertyReply rep;
-    long                    nbytes, rbytes;
+    unsigned long           nbytes, rbytes;
+    int                     ret = Success;
 
     XExtDisplayInfo *info = XInput_find_display(dpy);
 
@@ -81,30 +83,43 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
 	 * data, but this last byte is null terminated and convenient for
 	 * returning string properties, so the client doesn't then have to
 	 * recopy the string to make it null terminated.
+	 *
+	 * Maximum item limits are set to both prevent integer overflow when
+	 * calculating the amount of memory to malloc, and to limit how much
+	 * memory will be used if a server provides an insanely high count.
 	 */
 	switch (rep.format) {
 	case 8:
-	    nbytes = rep.nItems;
-	    rbytes = rep.nItems + 1;
-	    if (rbytes > 0 &&
-		(*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
-		_XReadPad (dpy, (char *) *prop, nbytes);
+	    if (rep.nItems < INT_MAX) {
+		nbytes = rep.nItems;
+		rbytes = rep.nItems + 1;
+		if ((*prop = Xmalloc (rbytes)))
+		    _XReadPad (dpy, (char *) *prop, nbytes);
+		else
+		    ret = BadAlloc;
+	    }
 	    break;
 
 	case 16:
-	    nbytes = rep.nItems << 1;
-	    rbytes = rep.nItems * sizeof (short) + 1;
-	    if (rbytes > 0 &&
-		(*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
-		_XRead16Pad (dpy, (short *) *prop, nbytes);
+	    if (rep.nItems < (INT_MAX / sizeof (short))) {
+		nbytes = rep.nItems << 1;
+		rbytes = rep.nItems * sizeof (short) + 1;
+		if ((*prop = Xmalloc (rbytes)))
+		    _XRead16Pad (dpy, (short *) *prop, nbytes);
+		else
+		    ret = BadAlloc;
+	    }
 	    break;
 
 	case 32:
-	    nbytes = rep.nItems << 2;
-	    rbytes = rep.nItems * sizeof (long) + 1;
-	    if (rbytes > 0 &&
-		(*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
-		_XRead32 (dpy, (long *) *prop, nbytes);
+	    if (rep.nItems < (INT_MAX / sizeof (long))) {
+		nbytes = rep.nItems << 2;
+		rbytes = rep.nItems * sizeof (long) + 1;
+		if ((*prop = Xmalloc (rbytes)))
+		    _XRead32 (dpy, (long *) *prop, nbytes);
+		else
+		    ret = BadAlloc;
+	    }
 	    break;
 
 	default:
@@ -112,16 +127,13 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
 	     * This part of the code should never be reached.  If it is,
 	     * the server sent back a property with an invalid format.
 	     */
-	    _XEatDataWords(dpy, rep.length);
-	    UnlockDisplay(dpy);
-	    SyncHandle();
-	    return(BadImplementation);
+	    ret = BadImplementation;
 	}
 	if (! *prop) {
 	    _XEatDataWords(dpy, rep.length);
-	    UnlockDisplay(dpy);
-	    SyncHandle();
-	    return(BadAlloc);
+	    if (ret == Success)
+		ret = BadAlloc;
+	    goto out;
 	}
 	(*prop)[rbytes - 1] = '\0';
     }
@@ -130,9 +142,10 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
     *actual_format = rep.format;
     *nitems = rep.nItems;
     *bytes_after = rep.bytesAfter;
+  out:
     UnlockDisplay (dpy);
     SyncHandle ();
 
-    return Success;
+    return ret;
 }
 

commit 528419b9ef437e7eeafb41bf45e8ff7d818bd845
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XIGetSelectedEvents() [CVE-2013-1984 6/8]
    
    If the number of events or masks reported by the server is large enough
    that it overflows when multiplied by the size of the appropriate struct,
    or the sizes overflow as they are totaled up, then memory corruption can
    occur when more bytes are copied from the X server reply than the size
    of the buffer we allocated to hold them.
    
    v2: check that reply size fits inside the data read from the server,
        so that we don't read out of bounds either
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XISelEv.c b/src/XISelEv.c
index f871222..0471bef 100644
--- a/src/XISelEv.c
+++ b/src/XISelEv.c
@@ -42,6 +42,7 @@ in this Software without prior written authorization from the author.
 #include <X11/extensions/ge.h>
 #include <X11/extensions/geproto.h>
 #include "XIint.h"
+#include <limits.h>
 
 int
 XISelectEvents(Display* dpy, Window win, XIEventMask* masks, int num_masks)
@@ -101,13 +102,14 @@ out:
 XIEventMask*
 XIGetSelectedEvents(Display* dpy, Window win, int *num_masks_return)
 {
-    int i, len = 0;
+    unsigned int i, len = 0;
     unsigned char *mask;
     XIEventMask *mask_out = NULL;
     xXIEventMask *mask_in = NULL, *mi;
     xXIGetSelectedEventsReq *req;
     xXIGetSelectedEventsReply reply;
     XExtDisplayInfo *info = XInput_find_display(dpy);
+    size_t rbytes;
 
     *num_masks_return = -1;
     LockDisplay(dpy);
@@ -129,11 +131,16 @@ XIGetSelectedEvents(Display* dpy, Window win, int *num_masks_return)
         goto out;
     }
 
-    mask_in = Xmalloc(reply.length * 4);
-    if (!mask_in)
+    if (reply.length < (INT_MAX >> 2)) {
+        rbytes = (unsigned long) reply.length << 2;
+        mask_in = Xmalloc(rbytes);
+    }
+    if (!mask_in) {
+        _XEatDataWords(dpy, reply.length);
         goto out;
+    }
 
-    _XRead(dpy, (char*)mask_in, reply.length * 4);
+    _XRead(dpy, (char*)mask_in, rbytes);
 
     /*
      * This function takes interleaved xXIEventMask structs & masks off
@@ -148,8 +155,14 @@ XIGetSelectedEvents(Display* dpy, Window win, int *num_masks_return)
 
     for (i = 0, mi = mask_in; i < reply.num_masks; i++)
     {
-        len += mi->mask_len * 4;
-        mi = (xXIEventMask*)((char*)mi + mi->mask_len * 4);
+        unsigned int mask_bytes = mi->mask_len * 4;
+        len += mask_bytes;
+        if (len > INT_MAX)
+            goto out;
+        if ((sizeof(xXIEventMask) + mask_bytes) > rbytes)
+            goto out;
+        rbytes -= (sizeof(xXIEventMask) + mask_bytes);
+        mi = (xXIEventMask*)((char*)mi + mask_bytes);
         mi++;
     }
 

commit 242f92b490a695fbab244af5bad11b71f897c732
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XIGetProperty() [CVE-2013-1984 5/8]
    
    If the number of items reported by the server is large enough that
    it overflows when multiplied by the size of the appropriate item type,
    then memory corruption can occur when more bytes are copied from the
    X server reply than the size of the buffer we allocated to hold them.
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XIProperties.c b/src/XIProperties.c
index 5e58fb6..32436d1 100644
--- a/src/XIProperties.c
+++ b/src/XIProperties.c
@@ -38,6 +38,7 @@
 #include <X11/extensions/XInput2.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 Atom*
 XIListProperties(Display* dpy, int deviceid, int *num_props_return)
@@ -170,7 +171,7 @@ XIGetProperty(Display* dpy, int deviceid, Atom property, long offset,
 {
     xXIGetPropertyReq   *req;
     xXIGetPropertyReply rep;
-    long                    nbytes, rbytes;
+    unsigned long       nbytes, rbytes;
 
     XExtDisplayInfo *info = XInput_find_display(dpy);
 
@@ -216,9 +217,11 @@ XIGetProperty(Display* dpy, int deviceid, Atom property, long offset,
 	 * recopy the string to make it null terminated.
 	 */
 
-        nbytes = rep.num_items * rep.format/8;
-        rbytes = nbytes + 1;
-        *data = Xmalloc(rbytes);
+	if (rep.num_items < (INT_MAX / (rep.format/8))) {
+	    nbytes = rep.num_items * rep.format/8;
+	    rbytes = nbytes + 1;
+	    *data = Xmalloc(rbytes);
+	}
 
 	if (!(*data)) {
 	    _XEatDataWords(dpy, rep.length);

commit bb922ed4253b35590f0369f32a917ff89ade0830
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XGetDeviceMotionEvents() [CVE-2013-1984 4/8]
    
    If the number of events or axes reported by the server is large enough
    that it overflows when multiplied by the size of the appropriate struct,
    then memory corruption can occur when more bytes are copied from the
    X server reply than the size of the buffer we allocated to hold them.
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGMotion.c b/src/XGMotion.c
index 5feac85..a4c75b6 100644
--- a/src/XGMotion.c
+++ b/src/XGMotion.c
@@ -59,6 +59,7 @@ SOFTWARE.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 XDeviceTimeCoord *
 XGetDeviceMotionEvents(
@@ -74,7 +75,7 @@ XGetDeviceMotionEvents(
     xGetDeviceMotionEventsReply rep;
     XDeviceTimeCoord *tc;
     int *data, *bufp, *readp, *savp;
-    long size, size2;
+    unsigned long size;
     int i, j;
     XExtDisplayInfo *info = XInput_find_display(dpy);
 
@@ -104,10 +105,21 @@ XGetDeviceMotionEvents(
 	SyncHandle();
 	return (NULL);
     }
-    size = rep.length << 2;
-    size2 = rep.nEvents * (sizeof(XDeviceTimeCoord) + (rep.axes * sizeof(int)));
-    savp = readp = (int *)Xmalloc(size);
-    bufp = (int *)Xmalloc(size2);
+    if (rep.length < (INT_MAX >> 2)) {
+	size = rep.length << 2;
+	savp = readp = Xmalloc(size);
+    } else {
+	size = 0;
+	savp = readp = NULL;
+    }
+    /* rep.axes is a CARD8, so assume max number of axes for bounds check */
+    if (rep.nEvents <
+	(INT_MAX / (sizeof(XDeviceTimeCoord) + (UCHAR_MAX * sizeof(int))))) {
+	size_t bsize = rep.nEvents *
+	    (sizeof(XDeviceTimeCoord) + (rep.axes * sizeof(int)));
+	bufp = Xmalloc(bsize);
+    } else
+	bufp = NULL;
     if (!bufp || !savp) {
 	Xfree(bufp);
 	Xfree(savp);

commit 6dd6dc51a2935c72774be81e5cc2ba2c30e9feff
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XGetDeviceDontPropagateList() [CVE-2013-1984 3/8]
    
    If the number of event classes reported by the server is large enough
    that it overflows when multiplied by the size of the appropriate struct,
    then memory corruption can occur when more bytes are copied from the
    X server reply than the size of the buffer we allocated to hold them.
    
    V2: EatData if count is 0 but length is > 0 to avoid XIOErrors
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetProp.c b/src/XGetProp.c
index 34bc581..b49328c 100644
--- a/src/XGetProp.c
+++ b/src/XGetProp.c
@@ -60,6 +60,7 @@ SOFTWARE.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 XEventClass *
 XGetDeviceDontPropagateList(
@@ -88,10 +89,11 @@ XGetDeviceDontPropagateList(
     }
     *count = rep.count;
 
-    if (*count) {
-	list = (XEventClass *) Xmalloc(rep.length * sizeof(XEventClass));
+    if (rep.length != 0) {
+	if ((rep.count != 0) && (rep.length < (INT_MAX / sizeof(XEventClass))))
+	    list = Xmalloc(rep.length * sizeof(XEventClass));
 	if (list) {
-	    int i;
+	    unsigned int i;
 	    CARD32 ec;
 
 	    /* read and assign each XEventClass separately because

commit 322ee3576789380222d4403366e4fd12fb24cb6a
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XGetFeedbackControl() [CVE-2013-1984 2/8]
    
    If the number of feedbacks reported by the server is large enough that
    it overflows when multiplied by the size of the appropriate struct, or
    if the total size of all the feedback structures overflows when added
    together, then memory corruption can occur when more bytes are copied from
    the X server reply than the size of the buffer we allocated to hold them.
    
    v2: check that reply size fits inside the data read from the server, so
        we don't read out of bounds either
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetFCtl.c b/src/XGetFCtl.c
index 28fab4d..bb50bf3 100644
--- a/src/XGetFCtl.c
+++ b/src/XGetFCtl.c
@@ -61,6 +61,7 @@ SOFTWARE.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 XFeedbackState *
 XGetFeedbackControl(
@@ -68,8 +69,6 @@ XGetFeedbackControl(
     XDevice		*dev,
     int			*num_feedbacks)
 {
-    int size = 0;
-    int nbytes, i;
     XFeedbackState *Feedback = NULL;
     XFeedbackState *Sav = NULL;
     xFeedbackState *f = NULL;
@@ -91,9 +90,16 @@ XGetFeedbackControl(
 	goto out;
 
     if (rep.length > 0) {
+	unsigned long nbytes;
+	size_t size = 0;
+	int i;
+
 	*num_feedbacks = rep.num_feedbacks;
-	nbytes = (long)rep.length << 2;
-	f = (xFeedbackState *) Xmalloc((unsigned)nbytes);
+
+	if (rep.length < (INT_MAX >> 2)) {
+	    nbytes = rep.length << 2;
+	    f = Xmalloc(nbytes);
+	}
 	if (!f) {
 	    _XEatDataWords(dpy, rep.length);
 	    goto out;
@@ -102,6 +108,10 @@ XGetFeedbackControl(
 	_XRead(dpy, (char *)f, nbytes);
 
 	for (i = 0; i < *num_feedbacks; i++) {
+	    if (f->length > nbytes)
+		goto out;
+	    nbytes -= f->length;
+
 	    switch (f->class) {
 	    case KbdFeedbackClass:
 		size += sizeof(XKbdFeedbackState);
@@ -116,6 +126,8 @@ XGetFeedbackControl(
 	    {
 		xStringFeedbackState *strf = (xStringFeedbackState *) f;
 
+		if (strf->num_syms_supported >= (INT_MAX / sizeof(KeySym)))
+		    goto out;
 		size += sizeof(XStringFeedbackState) +
 		    (strf->num_syms_supported * sizeof(KeySym));
 	    }
@@ -130,10 +142,12 @@ XGetFeedbackControl(
 		size += f->length;
 		break;
 	    }
+	    if (size > INT_MAX)
+		goto out;
 	    f = (xFeedbackState *) ((char *)f + f->length);
 	}
 
-	Feedback = (XFeedbackState *) Xmalloc((unsigned)size);
+	Feedback = Xmalloc(size);
 	if (!Feedback)
 	    goto out;
 

commit b0b13c12a8079a5a0e7f43b2b8983699057b2cec
Author: Alan Coopersmith <alan.coopersmith@oracle.com>
Date:   Sat Mar 9 22:55:23 2013 -0800

    integer overflow in XGetDeviceControl() [CVE-2013-1984 1/8]
    
    If the number of valuators reported by the server is large enough that
    it overflows when multiplied by the size of the appropriate struct, then
    memory corruption can occur when more bytes are copied from the X server
    reply than the size of the buffer we allocated to hold them.
    
    v2: check that reply size fits inside the data read from the server, so
    we don't read out of bounds either
    
    Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
    Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
    Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/XGetDCtl.c b/src/XGetDCtl.c
index f73a4e8..51ed0ae 100644
--- a/src/XGetDCtl.c
+++ b/src/XGetDCtl.c
@@ -61,6 +61,7 @@ SOFTWARE.
 #include <X11/extensions/XInput.h>
 #include <X11/extensions/extutil.h>
 #include "XIint.h"
+#include <limits.h>
 
 XDeviceControl *
 XGetDeviceControl(
@@ -68,8 +69,6 @@ XGetDeviceControl(
     XDevice		*dev,
     int			 control)
 {
-    int size = 0;
-    int nbytes, i;
     XDeviceControl *Device = NULL;
     XDeviceControl *Sav = NULL;
     xDeviceState *d = NULL;
@@ -92,8 +91,12 @@ XGetDeviceControl(
 	goto out;
 
     if (rep.length > 0) {
-	nbytes = (long)rep.length << 2;
-	d = (xDeviceState *) Xmalloc((unsigned)nbytes);
+	unsigned long nbytes;
+	size_t size = 0;


Reply to: