configure.ac | 9 +++++- src/dmx.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 82 insertions(+), 11 deletions(-)
New commits: commit 76e841968ceb69095eb0efcd435fc47440e86d2c Author: Alan Coopersmith <[email protected]> Date: Tue May 28 16:45:02 2013 -0700 libdmx 1.1.3 Signed-off-by: Alan Coopersmith <[email protected]> diff --git a/configure.ac b/configure.ac index 4629cf8..56805ce 100644 --- a/configure.ac +++ b/configure.ac @@ -21,7 +21,7 @@ # Initialize Autoconf AC_PREREQ([2.60]) -AC_INIT([libdmx], [1.1.2], +AC_INIT([libdmx], [1.1.3], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg], [libdmx]) AC_CONFIG_SRCDIR([Makefile.am]) AC_CONFIG_HEADERS([config.h]) commit 5074d9d64192bd04519a438062b7d5bf216d06ee Author: Alan Coopersmith <[email protected]> Date: Sat Mar 9 13:48:28 2013 -0800 integer overflow in DMXGetInputAttributes() [CVE-2013-1992 3/3] If the server provided nameLength causes integer overflow when padding length is added, a smaller buffer would be allocated than the amount of data written to it. Reported-by: Ilja Van Sprundel <[email protected]> Signed-off-by: Alan Coopersmith <[email protected]> diff --git a/src/dmx.c b/src/dmx.c index 67434c8..d097062 100644 --- a/src/dmx.c +++ b/src/dmx.c @@ -723,6 +723,7 @@ Bool DMXGetInputAttributes(Display *dpy, int id, DMXInputAttributes *inf) xDMXGetInputAttributesReply rep; xDMXGetInputAttributesReq *req; char *buffer; + Bool ret = False; DMXCheckExtension(dpy, info, False); @@ -737,6 +738,16 @@ Bool DMXGetInputAttributes(Display *dpy, int id, DMXInputAttributes *inf) return False; } + if (rep.nameLength < 1024) + buffer = Xmalloc(rep.nameLength + 1 + 4 /* for pad */); + else + buffer = NULL; /* name length is unbelievable, reject */ + + if (buffer == NULL) { + _XEatDataWords(dpy, rep.length); + goto end; + } + switch (rep.inputType) { case 0: inf->inputType = DMXLocalInputType; break; case 1: inf->inputType = DMXConsoleInputType; break; @@ -748,13 +759,14 @@ Bool DMXGetInputAttributes(Display *dpy, int id, DMXInputAttributes *inf) inf->isCore = rep.isCore; inf->sendsCore = rep.sendsCore; inf->detached = rep.detached; - buffer = Xmalloc(rep.nameLength + 1 + 4 /* for pad */); _XReadPad(dpy, buffer, rep.nameLength); buffer[rep.nameLength] = '\0'; inf->name = buffer; + ret = True; + end: UnlockDisplay(dpy); SyncHandle(); - return True; + return ret; } /** Add input. */ commit b6fe1a7af34ea620e002fc453f9c5eacf7db3969 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 9 13:48:28 2013 -0800 integer overflow in DMXGetWindowAttributes() [CVE-2013-1992 2/3] If the server provided screenCount causes integer overflow when multiplied by the size of each array element, a smaller buffer would be allocated than the amount of data written to it. Reported-by: Ilja Van Sprundel <[email protected]> Signed-off-by: Alan Coopersmith <[email protected]> diff --git a/src/dmx.c b/src/dmx.c index 15a6650..67434c8 100644 --- a/src/dmx.c +++ b/src/dmx.c @@ -524,6 +524,7 @@ Bool DMXGetWindowAttributes(Display *dpy, Window window, CARD32 *windows; /* Must match protocol size */ XRectangle *pos; /* Must match protocol size */ XRectangle *vis; /* Must match protocol size */ + Bool ret = False; DMXCheckExtension(dpy, info, False); @@ -538,11 +539,30 @@ Bool DMXGetWindowAttributes(Display *dpy, Window window, return False; } - /* FIXME: check for NULL? */ - screens = Xmalloc(rep.screenCount * sizeof(*screens)); - windows = Xmalloc(rep.screenCount * sizeof(*windows)); - pos = Xmalloc(rep.screenCount * sizeof(*pos)); - vis = Xmalloc(rep.screenCount * sizeof(*vis)); + /* + * rep.screenCount is a CARD32 so could be as large as 2^32 + * The X11 protocol limits the total screen size to 64k x 64k, + * and no screen can be smaller than a pixel. While technically + * that means we could theoretically reach 2^32 screens, and that's + * not even taking overlap into account, 64k seems far larger than + * any reasonable configuration, so we limit to that to prevent both + * integer overflow in the size calculations, and bad X server + * responses causing massive memory allocation. + */ + if (rep.screenCount < 65536) { + screens = Xmalloc(rep.screenCount * sizeof(*screens)); + windows = Xmalloc(rep.screenCount * sizeof(*windows)); + pos = Xmalloc(rep.screenCount * sizeof(*pos)); + vis = Xmalloc(rep.screenCount * sizeof(*vis)); + } else { + screens = windows = NULL; + pos = vis = NULL; + } + + if (!screens || !windows || !pos || !vis) { + _XEatDataWords(dpy, rep.length); + goto end; + } _XRead(dpy, (char *)screens, rep.screenCount * sizeof(*screens)); _XRead(dpy, (char *)windows, rep.screenCount * sizeof(*windows)); @@ -558,7 +578,9 @@ Bool DMXGetWindowAttributes(Display *dpy, Window window, inf->pos = pos[current]; inf->vis = vis[current]; } + ret = True; + end: Xfree(vis); Xfree(pos); Xfree(windows); @@ -566,7 +588,7 @@ Bool DMXGetWindowAttributes(Display *dpy, Window window, UnlockDisplay(dpy); SyncHandle(); - return True; + return ret; } /** If the DMXGetDesktopAttributes protocol request returns information commit 78e11efe70d00063c830475eaaaa42f19380755d Author: Alan Coopersmith <[email protected]> Date: Sat Mar 9 13:48:28 2013 -0800 integer overflow in DMXGetScreenAttributes() [CVE-2013-1992 1/3] If the server provided displayNameLength causes integer overflow when padding length is added, a smaller buffer would be allocated than the amount of data written to it. Reported-by: Ilja Van Sprundel <[email protected]> Signed-off-by: Alan Coopersmith <[email protected]> diff --git a/src/dmx.c b/src/dmx.c index e43d624..15a6650 100644 --- a/src/dmx.c +++ b/src/dmx.c @@ -250,6 +250,7 @@ Bool DMXGetScreenAttributes(Display *dpy, int physical_screen, XExtDisplayInfo *info = find_display(dpy); xDMXGetScreenAttributesReply rep; xDMXGetScreenAttributesReq *req; + Bool ret = False; DMXCheckExtension(dpy, info, False); @@ -264,7 +265,15 @@ Bool DMXGetScreenAttributes(Display *dpy, int physical_screen, SyncHandle(); return False; } - attr->displayName = Xmalloc(rep.displayNameLength + 1 + 4 /* for pad */); + + if (rep.displayNameLength < 1024) + attr->displayName = Xmalloc(rep.displayNameLength + 1 + 4 /* for pad */); + else + attr->displayName = NULL; /* name length is unbelievable, reject */ + if (attr->displayName == NULL) { + _XEatDataWords(dpy, rep.length); + goto end; + } _XReadPad(dpy, attr->displayName, rep.displayNameLength); attr->displayName[rep.displayNameLength] = '\0'; attr->logicalScreen = rep.logicalScreen; @@ -280,9 +289,13 @@ Bool DMXGetScreenAttributes(Display *dpy, int physical_screen, attr->rootWindowYoffset = rep.rootWindowYoffset; attr->rootWindowXorigin = rep.rootWindowXorigin; attr->rootWindowYorigin = rep.rootWindowYorigin; + + ret = True; + + end: UnlockDisplay(dpy); SyncHandle(); - return True; + return ret; } static CARD32 _DMXGetScreenAttribute(int bit, DMXScreenAttributes *attr) commit f34f6f64698c3b957aadba7315bb13726e3d79b0 Author: Alan Coopersmith <[email protected]> Date: Fri May 3 23:10:47 2013 -0700 Use _XEatDataWords to avoid overflow of rep.length bit shifting rep.length is a CARD32, so rep.length << 2 could overflow in 32-bit builds Signed-off-by: Alan Coopersmith <[email protected]> diff --git a/configure.ac b/configure.ac index 24e03fc..4629cf8 100644 --- a/configure.ac +++ b/configure.ac @@ -43,6 +43,13 @@ XORG_CHECK_MALLOC_ZERO # Obtain compiler/linker options for depedencies PKG_CHECK_MODULES(DMX, x11 xext xextproto [dmxproto >= 2.2.99.1]) +# Check for _XEatDataWords function that may be patched into older Xlib releases +SAVE_LIBS="$LIBS" +LIBS="$DMX_LIBS" +AC_CHECK_FUNCS([_XEatDataWords]) +LIBS="$SAVE_LIBS" + + AC_CONFIG_FILES([Makefile src/Makefile man/Makefile diff --git a/src/dmx.c b/src/dmx.c index 201568e..e43d624 100644 --- a/src/dmx.c +++ b/src/dmx.c @@ -38,12 +38,16 @@ * can be included in client applications by linking with the libdmx.a * library. */ +#ifdef HAVE_CONFIG_H +# include "config.h" +#endif #include <X11/Xlibint.h> #include <X11/extensions/Xext.h> #define EXTENSION_PROC_ARGS void * #include <X11/extensions/extutil.h> #include <X11/extensions/dmxproto.h> #include <X11/extensions/dmxext.h> +#include <limits.h> static XExtensionInfo dmx_extension_info_data; static XExtensionInfo *dmx_extension_info = &dmx_extension_info_data; @@ -82,6 +86,19 @@ static XEXT_GENERATE_FIND_DISPLAY(find_display, dmx_extension_info, static XEXT_GENERATE_CLOSE_DISPLAY(close_display, dmx_extension_info) +#ifndef HAVE__XEATDATAWORDS +#include <X11/Xmd.h> /* for LONG64 on 64-bit platforms */ + +static inline void _XEatDataWords(Display *dpy, unsigned long n) +{ +# ifndef LONG64 + if (n >= (ULONG_MAX >> 2)) + _XIOError(dpy); +# endif + _XEatData (dpy, n << 2); +} +#endif + /***************************************************************************** * * -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
