1665N/AFrom 19a9cd607de73947fcfb104682f203ffe4e1f4e5 Mon Sep 17 00:00:00 2001

1665N/AFrom: Tobias Stoeckmann <tobias@stoeckmann.org>

1665N/ADate: Sun, 25 Sep 2016 22:31:34 +0200

1665N/ASubject: [PATCH:libXi] Properly validate server responses.

1665N/A

1665N/ABy validating length fields from server responses, out of boundary

1665N/Aaccesses and endless loops can be mitigated.

1665N/A

1665N/ASigned-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

1665N/AReviewed-by: Matthieu Herrb <matthieu@herrb.eu>

1665N/A---

1665N/A src/XGMotion.c | 3 ++-

1665N/A src/XGetBMap.c | 3 ++-

1665N/A src/XGetDCtl.c | 6 ++++--

1665N/A src/XGetFCtl.c | 7 ++++++-

1665N/A src/XGetKMap.c | 14 +++++++++++---

1665N/A src/XGetMMap.c | 11 +++++++++--

1665N/A src/XIQueryDevice.c | 36 ++++++++++++++++++++++++++++++++++--

1665N/A src/XListDev.c | 21 +++++++++++++++------

1665N/A src/XOpenDev.c | 13 ++++++++++---

1665N/A src/XQueryDv.c | 8 ++++++--

1665N/A 10 files changed, 99 insertions(+), 23 deletions(-)

1665N/A

1665N/Adiff --git a/src/XGMotion.c b/src/XGMotion.c

1665N/Aindex 7785843..9433e29 100644

1665N/A--- a/src/XGMotion.c

1665N/A+++ b/src/XGMotion.c

1665N/A@@ -114,7 +114,8 @@ XGetDeviceMotionEvents(

1665N/A }

1665N/A /* rep.axes is a CARD8, so assume max number of axes for bounds check */

1665N/A if (rep.nEvents <

1665N/A- (INT_MAX / (sizeof(XDeviceTimeCoord) + (UCHAR_MAX * sizeof(int))))) {

1665N/A+ (INT_MAX / (sizeof(XDeviceTimeCoord) + (UCHAR_MAX * sizeof(int)))) &&

1665N/A+ rep.nEvents * (rep.axes + 1) <= rep.length) {

1665N/A size_t bsize = rep.nEvents *

1665N/A (sizeof(XDeviceTimeCoord) + (rep.axes * sizeof(int)));

1665N/A bufp = Xmalloc(bsize);

1665N/Adiff --git a/src/XGetBMap.c b/src/XGetBMap.c

1665N/Aindex 002daba..13bb8c6 100644

1665N/A--- a/src/XGetBMap.c

1665N/A+++ b/src/XGetBMap.c

1665N/A@@ -92,7 +92,8 @@ XGetDeviceButtonMapping(

1665N/A

1665N/A status = _XReply(dpy, (xReply *) & rep, 0, xFalse);

1665N/A if (status == 1) {

1665N/A- if (rep.length <= (sizeof(mapping) >> 2)) {

1665N/A+ if (rep.length <= (sizeof(mapping) >> 2) &&

1665N/A+ rep.nElts <= (rep.length << 2)) {

1665N/A unsigned long nbytes = rep.length << 2;

1665N/A _XRead(dpy, (char *)mapping, nbytes);

1665N/A

1665N/Adiff --git a/src/XGetDCtl.c b/src/XGetDCtl.c

1665N/Aindex c5d3b53..7f6b396 100644

1665N/A--- a/src/XGetDCtl.c

1665N/A+++ b/src/XGetDCtl.c

1665N/A@@ -93,7 +93,8 @@ XGetDeviceControl(

1665N/A if (rep.length > 0) {

1665N/A unsigned long nbytes;

1665N/A size_t size = 0;

1665N/A- if (rep.length < (INT_MAX >> 2)) {

1665N/A+ if (rep.length < (INT_MAX >> 2) &&

1665N/A+ (rep.length << 2) >= sizeof(xDeviceState)) {

1665N/A nbytes = (unsigned long) rep.length << 2;

1665N/A d = Xmalloc(nbytes);

1665N/A }

1665N/A@@ -117,7 +118,8 @@ XGetDeviceControl(

1665N/A size_t val_size;

1665N/A

1665N/A r = (xDeviceResolutionState *) d;

1665N/A- if (r->num_valuators >= (INT_MAX / (3 * sizeof(int))))

1665N/A+ if (sizeof(xDeviceResolutionState) > nbytes ||

1665N/A+ r->num_valuators >= (INT_MAX / (3 * sizeof(int))))

1665N/A goto out;

1665N/A val_size = 3 * sizeof(int) * r->num_valuators;

1665N/A if ((sizeof(xDeviceResolutionState) + val_size) > nbytes)

1665N/Adiff --git a/src/XGetFCtl.c b/src/XGetFCtl.c

1665N/Aindex 7fd6d0e..82dcc64 100644

1665N/A--- a/src/XGetFCtl.c

1665N/A+++ b/src/XGetFCtl.c

1665N/A@@ -73,6 +73,7 @@ XGetFeedbackControl(

1665N/A XFeedbackState *Sav = NULL;

1665N/A xFeedbackState *f = NULL;

1665N/A xFeedbackState *sav = NULL;

1665N/A+ char *end = NULL;

1665N/A xGetFeedbackControlReq *req;

1665N/A xGetFeedbackControlReply rep;

1665N/A XExtDisplayInfo *info = XInput_find_display(dpy);

1665N/A@@ -105,10 +106,12 @@ XGetFeedbackControl(

1665N/A goto out;

1665N/A }

1665N/A sav = f;

1665N/A+ end = (char *)f + nbytes;

1665N/A _XRead(dpy, (char *)f, nbytes);

1665N/A

1665N/A for (i = 0; i < *num_feedbacks; i++) {

1665N/A- if (f->length > nbytes)

1665N/A+ if ((char *)f + sizeof(*f) > end ||

1665N/A+ f->length == 0 || f->length > nbytes)

1665N/A goto out;

1665N/A nbytes -= f->length;

1665N/A

1665N/A@@ -125,6 +128,8 @@ XGetFeedbackControl(

1665N/A case StringFeedbackClass:

1665N/A {

1665N/A xStringFeedbackState *strf = (xStringFeedbackState *) f;

1665N/A+ if ((char *)f + sizeof(*strf) > end)

1665N/A+ goto out;

1665N/A size += sizeof(XStringFeedbackState) +

1665N/A (strf->num_syms_supported * sizeof(KeySym));

1665N/A }

1665N/Adiff --git a/src/XGetKMap.c b/src/XGetKMap.c

1665N/Aindex 0540ce4..008a72b 100644

1665N/A--- a/src/XGetKMap.c

1665N/A+++ b/src/XGetKMap.c

1665N/A@@ -54,6 +54,7 @@ SOFTWARE.

1665N/A #include <config.h>

1665N/A #endif

1665N/A

1665N/A+#include <limits.h>

1665N/A #include <X11/extensions/XI.h>

1665N/A #include <X11/extensions/XIproto.h>

1665N/A #include <X11/Xlibint.h>

1665N/A@@ -93,9 +94,16 @@ XGetDeviceKeyMapping(register Display * dpy, XDevice * dev,

1665N/A return (KeySym *) NULL;

1665N/A }

1665N/A if (rep.length > 0) {

1665N/A- *syms_per_code = rep.keySymsPerKeyCode;

1665N/A- nbytes = (long)rep.length << 2;

1665N/A- mapping = (KeySym *) Xmalloc((unsigned)nbytes);

1665N/A+ if (rep.length < INT_MAX >> 2 &&

1665N/A+ rep.length == rep.keySymsPerKeyCode * keycount) {

1665N/A+ *syms_per_code = rep.keySymsPerKeyCode;

1665N/A+ nbytes = (long)rep.length << 2;

1665N/A+ mapping = (KeySym *) Xmalloc((unsigned)nbytes);

1665N/A+ } else {

1665N/A+ *syms_per_code = 0;

1665N/A+ nbytes = 0;

1665N/A+ mapping = NULL;

1665N/A+ }

1665N/A if (mapping)

1665N/A _XRead(dpy, (char *)mapping, nbytes);

1665N/A else

1665N/Adiff --git a/src/XGetMMap.c b/src/XGetMMap.c

1665N/Aindex 246698c..33c114f 100644

1665N/A--- a/src/XGetMMap.c

1665N/A+++ b/src/XGetMMap.c

1665N/A@@ -53,6 +53,7 @@ SOFTWARE.

1665N/A #include <config.h>

1665N/A #endif

1665N/A

1665N/A+#include <limits.h>

1665N/A #include <X11/extensions/XI.h>

1665N/A #include <X11/extensions/XIproto.h>

1665N/A #include <X11/Xlibint.h>

1665N/A@@ -85,8 +86,14 @@ XGetDeviceModifierMapping(

1665N/A SyncHandle();

1665N/A return (XModifierKeymap *) NULL;

1665N/A }

1665N/A- nbytes = (unsigned long)rep.length << 2;

1665N/A- res = (XModifierKeymap *) Xmalloc(sizeof(XModifierKeymap));

1665N/A+ if (rep.length < (INT_MAX >> 2) &&

1665N/A+ rep.numKeyPerModifier == rep.length >> 1) {

1665N/A+ nbytes = (unsigned long)rep.length << 2;

1665N/A+ res = (XModifierKeymap *) Xmalloc(sizeof(XModifierKeymap));

1665N/A+ } else {

1665N/A+ nbytes = 0;

1665N/A+ res = NULL;

1665N/A+ }

1665N/A if (res) {

1665N/A res->modifiermap = (KeyCode *) Xmalloc(nbytes);

1665N/A if (res->modifiermap)

1665N/Adiff --git a/src/XIQueryDevice.c b/src/XIQueryDevice.c

1665N/Aindex fb8504f..a457cd6 100644

1665N/A--- a/src/XIQueryDevice.c

1665N/A+++ b/src/XIQueryDevice.c

1665N/A@@ -26,6 +26,7 @@

1665N/A #include <config.h>

1665N/A #endif

1665N/A

1665N/A+#include <limits.h>

1665N/A #include <stdint.h>

1665N/A #include <X11/Xlibint.h>

1665N/A #include <X11/extensions/XI2proto.h>

1665N/A@@ -43,6 +44,7 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)

1665N/A xXIQueryDeviceReq *req;

1665N/A xXIQueryDeviceReply reply;

1665N/A char *ptr;

1665N/A+ char *end;

1665N/A int i;

1665N/A char *buf;

1665N/A

1665N/A@@ -60,14 +62,24 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)

1665N/A if (!_XReply(dpy, (xReply*) &reply, 0, xFalse))

1665N/A goto error;

1665N/A

1665N/A- *ndevices_return = reply.num_devices;

1665N/A- info = Xmalloc((reply.num_devices + 1) * sizeof(XIDeviceInfo));

1665N/A+ if (reply.length < INT_MAX / 4)

1665N/A+ {

1665N/A+ *ndevices_return = reply.num_devices;

1665N/A+ info = Xmalloc((reply.num_devices + 1) * sizeof(XIDeviceInfo));

1665N/A+ }

1665N/A+ else

1665N/A+ {

1665N/A+ *ndevices_return = 0;

1665N/A+ info = NULL;

1665N/A+ }

1665N/A+

1665N/A if (!info)

1665N/A goto error;

1665N/A

1665N/A buf = Xmalloc(reply.length * 4);

1665N/A _XRead(dpy, buf, reply.length * 4);

1665N/A ptr = buf;

1665N/A+ end = buf + reply.length * 4;

1665N/A

1665N/A /* info is a null-terminated array */

1665N/A info[reply.num_devices].name = NULL;

1665N/A@@ -79,6 +91,9 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)

1665N/A XIDeviceInfo *lib = &info[i];

1665N/A xXIDeviceInfo *wire = (xXIDeviceInfo*)ptr;

1665N/A

1665N/A+ if (ptr + sizeof(xXIDeviceInfo) > end)

1665N/A+ goto error_loop;

1665N/A+

1665N/A lib->deviceid = wire->deviceid;

1665N/A lib->use = wire->use;

1665N/A lib->attachment = wire->attachment;

1665N/A@@ -87,12 +102,23 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)

1665N/A

1665N/A ptr += sizeof(xXIDeviceInfo);

1665N/A

1665N/A+ if (ptr + wire->name_len > end)

1665N/A+ goto error_loop;

1665N/A+

1665N/A lib->name = Xcalloc(wire->name_len + 1, 1);

1665N/A+ if (lib->name == NULL)

1665N/A+ goto error_loop;

1665N/A strncpy(lib->name, ptr, wire->name_len);

1665N/A+ lib->name[wire->name_len] = '\0';

1665N/A ptr += ((wire->name_len + 3)/4) * 4;

1665N/A

1665N/A sz = size_classes((xXIAnyInfo*)ptr, nclasses);

1665N/A lib->classes = Xmalloc(sz);

1665N/A+ if (lib->classes == NULL)

1665N/A+ {

1665N/A+ Xfree(lib->name);

1665N/A+ goto error_loop;

1665N/A+ }

1665N/A ptr += copy_classes(lib, (xXIAnyInfo*)ptr, &nclasses);

1665N/A /* We skip over unused classes */

1665N/A lib->num_classes = nclasses;

1665N/A@@ -103,6 +129,12 @@ XIQueryDevice(Display *dpy, int deviceid, int *ndevices_return)

1665N/A SyncHandle();

1665N/A return info;

1665N/A

1665N/A+error_loop:

1665N/A+ while (--i >= 0)

1665N/A+ {

1665N/A+ Xfree(info[i].name);

1665N/A+ Xfree(info[i].classes);

1665N/A+ }

1665N/A error:

1665N/A UnlockDisplay(dpy);

1665N/A error_unlocked:

1665N/Adiff --git a/src/XListDev.c b/src/XListDev.c

1665N/Aindex b85ff3c..f850cd0 100644

1665N/A--- a/src/XListDev.c

1665N/A+++ b/src/XListDev.c

1665N/A@@ -74,7 +74,7 @@ static int pad_to_xid(int base_size)

1665N/A }

1665N/A

1665N/A static size_t

1665N/A-SizeClassInfo(xAnyClassPtr *any, int num_classes)

1665N/A+SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)

1665N/A {

1665N/A int size = 0;

1665N/A int j;

1665N/A@@ -90,6 +90,8 @@ SizeClassInfo(xAnyClassPtr *any, int num_classes)

1665N/A {

1665N/A xValuatorInfoPtr v;

1665N/A

1665N/A+ if (len < sizeof(v))

1665N/A+ return 0;

1665N/A v = (xValuatorInfoPtr) *any;

1665N/A size += pad_to_xid(sizeof(XValuatorInfo) +

1665N/A (v->num_axes * sizeof(XAxisInfo)));

1665N/A@@ -98,6 +100,8 @@ SizeClassInfo(xAnyClassPtr *any, int num_classes)

1665N/A default:

1665N/A break;

1665N/A }

1665N/A+ if ((*any)->length > len)

1665N/A+ return 0;

1665N/A *any = (xAnyClassPtr) ((char *)(*any) + (*any)->length);

1665N/A }

1665N/A

1665N/A@@ -170,7 +174,7 @@ XListInputDevices(

1665N/A register Display *dpy,

1665N/A int *ndevices)

1665N/A {

1665N/A- size_t size;

1665N/A+ size_t s, size;

1665N/A xListInputDevicesReq *req;

1665N/A xListInputDevicesReply rep;

1665N/A xDeviceInfo *list, *slist = NULL;

1665N/A@@ -178,6 +182,7 @@ XListInputDevices(

1665N/A XDeviceInfo *clist = NULL;

1665N/A xAnyClassPtr any, sav_any;

1665N/A XAnyClassPtr Any;

1665N/A+ char *end = NULL;

1665N/A unsigned char *nptr, *Nptr;

1665N/A int i;

1665N/A unsigned long rlen;

1665N/A@@ -213,16 +218,20 @@ XListInputDevices(

1665N/A

1665N/A any = (xAnyClassPtr) ((char *)list + (*ndevices * sizeof(xDeviceInfo)));

1665N/A sav_any = any;

1665N/A+ end = (char *)list + rlen;

1665N/A for (i = 0; i < *ndevices; i++, list++) {

1665N/A- size += SizeClassInfo(&any, (int)list->num_classes);

1665N/A+ s = SizeClassInfo(&any, end - (char *)any, (int)list->num_classes);

1665N/A+ if (!s)

1665N/A+ goto out;

1665N/A+ size += s;

1665N/A }

1665N/A

1665N/A- Nptr = ((unsigned char *)list) + rlen + 1;

1665N/A+ Nptr = ((unsigned char *)list) + rlen;

1665N/A for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) {

1665N/A+ if (nptr >= Nptr)

1665N/A+ goto out;

1665N/A size += *nptr + 1;

1665N/A nptr += (*nptr + 1);

1665N/A- if (nptr > Nptr)

1665N/A- goto out;

1665N/A }

1665N/A

1665N/A clist = (XDeviceInfoPtr) Xmalloc(size);

1665N/Adiff --git a/src/XOpenDev.c b/src/XOpenDev.c

1665N/Aindex 029dec2..4b3c460 100644

1665N/A--- a/src/XOpenDev.c

1665N/A+++ b/src/XOpenDev.c

1665N/A@@ -53,6 +53,7 @@ SOFTWARE.

1665N/A #include <config.h>

1665N/A #endif

1665N/A

1665N/A+#include <limits.h>

1665N/A #include <X11/extensions/XI.h>

1665N/A #include <X11/extensions/XIproto.h>

1665N/A #include <X11/Xlibint.h>

1665N/A@@ -86,9 +87,15 @@ XOpenDevice(

1665N/A return (XDevice *) NULL;

1665N/A }

1665N/A

1665N/A- rlen = rep.length << 2;

1665N/A- dev = (XDevice *) Xmalloc(sizeof(XDevice) + rep.num_classes *

1665N/A- sizeof(XInputClassInfo));

1665N/A+ if (rep.length < INT_MAX >> 2 &&

1665N/A+ (rep.length << 2) >= rep.num_classes * sizeof(xInputClassInfo)) {

1665N/A+ rlen = rep.length << 2;

1665N/A+ dev = (XDevice *) Xmalloc(sizeof(XDevice) + rep.num_classes *

1665N/A+ sizeof(XInputClassInfo));

1665N/A+ } else {

1665N/A+ rlen = 0;

1665N/A+ dev = NULL;

1665N/A+ }

1665N/A if (dev) {

1665N/A int dlen; /* data length */

1665N/A

1665N/Adiff --git a/src/XQueryDv.c b/src/XQueryDv.c

1665N/Aindex de1c0e5..7ee2272 100644

1665N/A--- a/src/XQueryDv.c

1665N/A+++ b/src/XQueryDv.c

1665N/A@@ -73,7 +73,7 @@ XQueryDeviceState(

1665N/A xQueryDeviceStateReply rep;

1665N/A XDeviceState *state = NULL;

1665N/A XInputClass *any, *Any;

1665N/A- char *data = NULL;

1665N/A+ char *data = NULL, *end = NULL;

1665N/A XExtDisplayInfo *info = XInput_find_display(dpy);

1665N/A

1665N/A LockDisplay(dpy);

1665N/A@@ -92,6 +92,7 @@ XQueryDeviceState(

1665N/A if (rep.length < (INT_MAX >> 2)) {

1665N/A rlen = (unsigned long) rep.length << 2;

1665N/A data = Xmalloc(rlen);

1665N/A+ end = data + rlen;

1665N/A }

1665N/A if (!data) {

1665N/A _XEatDataWords(dpy, rep.length);

1665N/A@@ -100,7 +101,8 @@ XQueryDeviceState(

1665N/A _XRead(dpy, data, rlen);

1665N/A

1665N/A for (i = 0, any = (XInputClass *) data; i < (int)rep.num_classes; i++) {

1665N/A- if (any->length > rlen)

1665N/A+ if ((char *)any + sizeof(XInputClass) > end ||

1665N/A+ any->length == 0 || any->length > rlen)

1665N/A goto out;

1665N/A rlen -= any->length;

1665N/A

1665N/A@@ -114,6 +116,8 @@ XQueryDeviceState(

1665N/A case ValuatorClass:

1665N/A {

1665N/A xValuatorState *v = (xValuatorState *) any;

1665N/A+ if ((char *)any + sizeof(xValuatorState) > end)

1665N/A+ goto out;

1665N/A size += (sizeof(XValuatorState) +

1665N/A (v->num_valuators * sizeof(int)));

1665N/A }

1665N/A--

1665N/A2.7.4

1665N/A