On Sun, Sep 25, 2016 at 22:50:43 +0200, Matthieu Herrb wrote: > From: Tobias Stoeckmann <tob...@stoeckmann.org> > > By validating length fields from server responses, out of boundary > accesses and endless loops can be mitigated. > > Signed-off-by: Tobias Stoeckmann <tob...@stoeckmann.org> > Reviewed-by: Matthieu Herrb <matth...@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(-) > [...] > diff --git a/src/XIQueryDevice.c b/src/XIQueryDevice.c > index fb8504f..a457cd6 100644 > --- a/src/XIQueryDevice.c > +++ b/src/XIQueryDevice.c > @@ -26,6 +26,7 @@ > #include <config.h> > #endif > > +#include <limits.h> > #include <stdint.h> > #include <X11/Xlibint.h> > #include <X11/extensions/XI2proto.h> > @@ -43,6 +44,7 @@ XIQueryDevice(Display *dpy, int deviceid, int > *ndevices_return) > xXIQueryDeviceReq *req; > xXIQueryDeviceReply reply; > char *ptr; > + char *end; > int i; > char *buf; > > @@ -60,14 +62,24 @@ XIQueryDevice(Display *dpy, int deviceid, int > *ndevices_return) > if (!_XReply(dpy, (xReply*) &reply, 0, xFalse)) > goto error; > > - *ndevices_return = reply.num_devices; > - info = Xmalloc((reply.num_devices + 1) * sizeof(XIDeviceInfo)); > + if (reply.length < INT_MAX / 4) > + { > + *ndevices_return = reply.num_devices; > + info = Xmalloc((reply.num_devices + 1) * sizeof(XIDeviceInfo)); > + } > + else > + { > + *ndevices_return = 0; > + info = NULL; > + } > + > if (!info) > goto error; > > buf = Xmalloc(reply.length * 4);
Should we check for allocation failure here? > _XRead(dpy, buf, reply.length * 4); > ptr = buf; > + end = buf + reply.length * 4; > > /* info is a null-terminated array */ > info[reply.num_devices].name = NULL; > @@ -79,6 +91,9 @@ XIQueryDevice(Display *dpy, int deviceid, int > *ndevices_return) > XIDeviceInfo *lib = &info[i]; > xXIDeviceInfo *wire = (xXIDeviceInfo*)ptr; > > + if (ptr + sizeof(xXIDeviceInfo) > end) > + goto error_loop; > + > lib->deviceid = wire->deviceid; > lib->use = wire->use; > lib->attachment = wire->attachment; > @@ -87,12 +102,23 @@ XIQueryDevice(Display *dpy, int deviceid, int > *ndevices_return) > > ptr += sizeof(xXIDeviceInfo); > > + if (ptr + wire->name_len > end) > + goto error_loop; > + > lib->name = Xcalloc(wire->name_len + 1, 1); > + if (lib->name == NULL) > + goto error_loop; > strncpy(lib->name, ptr, wire->name_len); > + lib->name[wire->name_len] = '\0'; > ptr += ((wire->name_len + 3)/4) * 4; > > sz = size_classes((xXIAnyInfo*)ptr, nclasses); > lib->classes = Xmalloc(sz); > + if (lib->classes == NULL) > + { > + Xfree(lib->name); > + goto error_loop; > + } > ptr += copy_classes(lib, (xXIAnyInfo*)ptr, &nclasses); > /* We skip over unused classes */ > lib->num_classes = nclasses; > @@ -103,6 +129,12 @@ XIQueryDevice(Display *dpy, int deviceid, int > *ndevices_return) > SyncHandle(); > return info; > > +error_loop: > + while (--i >= 0) > + { > + Xfree(info[i].name); > + Xfree(info[i].classes); > + } > error: > UnlockDisplay(dpy); > error_unlocked: AFAICT this is missing Xfree(info); Xfree(buf); Cheers, Julien _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel