From 19a9cd607de73947fcfb104682f203ffe4e1f4e5 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Sun, 25 Sep 2016 22:31:34 +0200
Subject: [PATCH] Properly validate server responses.

By validating length fields from server responses, out of boundary
accesses and endless loops can be mitigated.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
 src/XGMotion.c      |  3 ++-
 src/XGetBMap.c      |  3 ++-
 src/XGetDCtl.c      |  6 ++++--
 src/XGetFCtl.c      |  7 ++++++-
 src/XGetKMap.c      | 14 +++++++++++---
 src/XGetMMap.c      | 11 +++++++++--
 src/XIQueryDevice.c | 36 ++++++++++++++++++++++++++++++++++--
 src/XListDev.c      | 21 +++++++++++++++------
 src/XOpenDev.c      | 13 ++++++++++---
 src/XQueryDv.c      |  8 ++++++--
 10 files changed, 99 insertions(+), 23 deletions(-)

Index: libXi-1.2.1/src/XGMotion.c
===================================================================
--- libXi-1.2.1.orig/src/XGMotion.c
+++ libXi-1.2.1/src/XGMotion.c
@@ -120,7 +120,8 @@ Time stop;
     }
     /* 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))))) {
+	(INT_MAX / (sizeof(XDeviceTimeCoord) + (UCHAR_MAX * sizeof(int)))) &&
+	rep.nEvents * (rep.axes + 1) <= rep.length) {
 	size_t bsize = rep.nEvents *
 	    (sizeof(XDeviceTimeCoord) + (rep.axes * sizeof(int)));
 	bufp = Xmalloc(bsize);
Index: libXi-1.2.1/src/XGetBMap.c
===================================================================
--- libXi-1.2.1.orig/src/XGetBMap.c
+++ libXi-1.2.1/src/XGetBMap.c
@@ -92,7 +92,8 @@ XGetDeviceButtonMapping(dpy, device, map
 
     status = _XReply(dpy, (xReply *) & rep, 0, xFalse);
     if (status == 1) {
-	if (rep.length <= (sizeof(mapping) >> 2)) {
+	if (rep.length <= (sizeof(mapping) >> 2) &&
+	    rep.nElts <= (rep.length << 2)) {
 	    unsigned long nbytes = rep.length << 2;
 	    _XRead(dpy, (char *)mapping, nbytes);
 
Index: libXi-1.2.1/src/XGetDCtl.c
===================================================================
--- libXi-1.2.1.orig/src/XGetDCtl.c
+++ libXi-1.2.1/src/XGetDCtl.c
@@ -94,7 +94,8 @@ XGetDeviceControl(dpy, dev, control)
     if (rep.length > 0) {
 	unsigned long nbytes;
 	size_t size = 0;
-	if (rep.length < (INT_MAX >> 2)) {
+	if (rep.length < (INT_MAX >> 2) &&
+	    (rep.length << 2) >= sizeof(xDeviceState)) {
 	    nbytes = (unsigned long) rep.length << 2;
 	    d = Xmalloc(nbytes);
 	}
@@ -120,7 +121,8 @@ XGetDeviceControl(dpy, dev, control)
 	    size_t val_size;
 
 	    r = (xDeviceResolutionState *) d;
-	    if (r->num_valuators >= (INT_MAX / (3 * sizeof(int))))
+	    if (sizeof(xDeviceResolutionState) > nbytes ||
+		r->num_valuators >= (INT_MAX / (3 * sizeof(int))))
 		goto out;
 	    val_size = 3 * sizeof(int) * r->num_valuators;
 	    if ((sizeof(xDeviceResolutionState) + val_size) > nbytes)
Index: libXi-1.2.1/src/XGetFCtl.c
===================================================================
--- libXi-1.2.1.orig/src/XGetFCtl.c
+++ libXi-1.2.1/src/XGetFCtl.c
@@ -72,6 +72,7 @@ XGetFeedbackControl(dpy, dev, num_feedba
     XFeedbackState *Sav = NULL;
     xFeedbackState *f = NULL;
     xFeedbackState *sav = NULL;
+    char *end = NULL;
     xGetFeedbackControlReq *req;
     xGetFeedbackControlReply rep;
     XExtDisplayInfo *info = XInput_find_display(dpy);
@@ -108,10 +109,12 @@ XGetFeedbackControl(dpy, dev, num_feedba
 	    return (XFeedbackState *) NULL;
 	}
 	sav = f;
+	end = (char *)f + nbytes;
 	_XRead(dpy, (char *)f, nbytes);
 
 	for (i = 0; i < *num_feedbacks; i++) {
-	    if (f->length > nbytes)
+	    if ((char *)f + sizeof(*f) > end ||
+	        f->length == 0 || f->length > nbytes)
 		goto out;
 	    nbytes -= f->length;
 
@@ -129,6 +132,8 @@ XGetFeedbackControl(dpy, dev, num_feedba
 	    {
 		xStringFeedbackState *strf = (xStringFeedbackState *) f;
 
+		if ((char *)f + sizeof(*strf) > end)
+		    goto out;
 		if (strf->num_syms_supported >= (INT_MAX / sizeof(KeySym)))
 		    goto out;
 		size += sizeof(XStringFeedbackState) +
Index: libXi-1.2.1/src/XGetKMap.c
===================================================================
--- libXi-1.2.1.orig/src/XGetKMap.c
+++ libXi-1.2.1/src/XGetKMap.c
@@ -53,6 +53,7 @@ SOFTWARE.
  *
  */
 
+#include <limits.h>
 #include <X11/extensions/XI.h>
 #include <X11/extensions/XIproto.h>
 #include <X11/Xlibint.h>
@@ -92,9 +93,16 @@ XGetDeviceKeyMapping(register Display *
 	return (KeySym *) NULL;
     }
     if (rep.length > 0) {
-	*syms_per_code = rep.keySymsPerKeyCode;
-	nbytes = (long)rep.length << 2;
-	mapping = (KeySym *) Xmalloc((unsigned)nbytes);
+	if (rep.length < INT_MAX >> 2 &&
+	    rep.length == rep.keySymsPerKeyCode * keycount) {
+	    *syms_per_code = rep.keySymsPerKeyCode;
+	    nbytes = (long)rep.length << 2;
+	    mapping = (KeySym *) Xmalloc((unsigned)nbytes);
+	} else {
+	    *syms_per_code = 0;
+	    nbytes = 0;
+	    mapping = NULL;
+	}
 	if (mapping)
 	    _XRead(dpy, (char *)mapping, nbytes);
 	else
Index: libXi-1.2.1/src/XGetMMap.c
===================================================================
--- libXi-1.2.1.orig/src/XGetMMap.c
+++ libXi-1.2.1/src/XGetMMap.c
@@ -53,6 +53,7 @@ SOFTWARE.
  *
  */
 
+#include <limits.h>
 #include <X11/extensions/XI.h>
 #include <X11/extensions/XIproto.h>
 #include <X11/Xlibint.h>
@@ -85,8 +86,14 @@ XGetDeviceModifierMapping(dpy, dev)
 	SyncHandle();
 	return (XModifierKeymap *) NULL;
     }
-    nbytes = (unsigned long)rep.length << 2;
-    res = (XModifierKeymap *) Xmalloc(sizeof(XModifierKeymap));
+    if (rep.length < (INT_MAX >> 2) &&
+	rep.numKeyPerModifier == rep.length >> 1) {
+	nbytes = (unsigned long)rep.length << 2;
+	res = (XModifierKeymap *) Xmalloc(sizeof(XModifierKeymap));
+    } else {
+	nbytes = 0;
+	res = NULL;
+    }
     if (res) {
 	res->modifiermap = (KeyCode *) Xmalloc(nbytes);
 	if (res->modifiermap)
Index: libXi-1.2.1/src/XListDev.c
===================================================================
--- libXi-1.2.1.orig/src/XListDev.c
+++ libXi-1.2.1/src/XListDev.c
@@ -137,12 +137,12 @@ XListInputDevices(dpy, ndevices)
 	    }
 	}
 
-	Nptr = ((unsigned char *)list) + rlen + 1;
+	Nptr = ((unsigned char *)list) + rlen;
 	for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) {
+	    if (nptr >= Nptr)
+		goto out;
 	    size += *nptr + 1;
 	    nptr += (*nptr + 1);
-	    if (nptr > Nptr)
-		goto out;
 	}
 
 	clist = (XDeviceInfoPtr) Xmalloc(size);
Index: libXi-1.2.1/src/XOpenDev.c
===================================================================
--- libXi-1.2.1.orig/src/XOpenDev.c
+++ libXi-1.2.1/src/XOpenDev.c
@@ -53,6 +53,7 @@ SOFTWARE.
  *
  */
 
+#include <limits.h>
 #include <X11/extensions/XI.h>
 #include <X11/extensions/XIproto.h>
 #include <X11/Xlibint.h>
@@ -86,9 +87,15 @@ XOpenDevice(dpy, id)
 	return (XDevice *) NULL;
     }
 
-    rlen = rep.length << 2;
-    dev = (XDevice *) Xmalloc(sizeof(XDevice) + rep.num_classes *
-			      sizeof(XInputClassInfo));
+    if (rep.length < INT_MAX >> 2 &&
+	(rep.length << 2) >= rep.num_classes * sizeof(xInputClassInfo)) {
+	rlen = rep.length << 2;
+	dev = (XDevice *) Xmalloc(sizeof(XDevice) + rep.num_classes *
+				  sizeof(XInputClassInfo));
+    } else {
+	rlen = 0;
+	dev = NULL;
+    }
     if (dev) {
 	int dlen;	/* data length */
 
Index: libXi-1.2.1/src/XQueryDv.c
===================================================================
--- libXi-1.2.1.orig/src/XQueryDv.c
+++ libXi-1.2.1/src/XQueryDv.c
@@ -73,7 +73,7 @@ XQueryDeviceState(dpy, dev)
     xQueryDeviceStateReply rep;
     XDeviceState *state = NULL;
     XInputClass *any, *Any;
-    char *data;
+    char *data = NULL, *end = NULL;
     XExtDisplayInfo *info = XInput_find_display(dpy);
 
     LockDisplay(dpy);
@@ -95,6 +95,7 @@ XQueryDeviceState(dpy, dev)
 	if (rep.length < (INT_MAX >> 2)) {
 	    rlen = (unsigned long) rep.length << 2;
 	    data = Xmalloc(rlen);
+	    end = data + rlen;
 	}
 	if (!data) {
 	    _XEatData(dpy, (unsigned long)rlen);
@@ -105,7 +106,8 @@ XQueryDeviceState(dpy, dev)
 	_XRead(dpy, data, rlen);
 
 	for (i = 0, any = (XInputClass *) data; i < (int)rep.num_classes; i++) {
-	    if (any->length > rlen)
+	    if ((char *)any + sizeof(XInputClass) > end ||
+		any->length == 0 || any->length > rlen)
 		goto out;
 	    rlen -= any->length;
 
@@ -119,6 +121,8 @@ XQueryDeviceState(dpy, dev)
 	    case ValuatorClass:
 	    {
 		xValuatorState *v = (xValuatorState *) any;
+		if ((char *)any + sizeof(xValuatorState) > end)
+		    goto out;
 		size += (sizeof(XValuatorState) +
 			 (v->num_valuators * sizeof(int)));
 	    }
