ChangeLog | 57 +++++++++++++++++++++++++++++++++++++ configure.ac | 9 +++++ debian/changelog | 15 +++++++++ debian/compat | 2 - debian/rules | 16 +--------- src/dmx.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++------- 6 files changed, 158 insertions(+), 25 deletions(-)
New commits: commit 044332a5deacbe957e935f10f455aa86996c6cab Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 12:58:06 2013 +0200 Upload to unstable diff --git a/debian/changelog b/debian/changelog index 0b0e13d..cd16b02 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,11 +1,11 @@ -libdmx (1:1.1.3-1) UNRELEASED; urgency=low +libdmx (1:1.1.3-1) unstable; urgency=low * New upstream release. * Use dpkg-buildflags. * Bump debhelper compat to 7. * Disable silent rules. - -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:19:24 +0200 + -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:57:42 +0200 libdmx (1:1.1.2-1+deb7u1) wheezy-security; urgency=high commit 4bbb7e1e14545221eb3ddc3daba7c87ad95c4a9d Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 12:52:02 2013 +0200 Disable silent rules. diff --git a/debian/changelog b/debian/changelog index c647840..0b0e13d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,7 @@ libdmx (1:1.1.3-1) UNRELEASED; urgency=low * New upstream release. * Use dpkg-buildflags. * Bump debhelper compat to 7. + * Disable silent rules. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:19:24 +0200 diff --git a/debian/rules b/debian/rules index 3bc7584..ce7e34e 100755 --- a/debian/rules +++ b/debian/rules @@ -36,6 +36,7 @@ build-stamp: ../configure --prefix=/usr --mandir=\$${prefix}/share/man \ --libdir=\$${prefix}/lib/$(DEB_HOST_MULTIARCH) \ --infodir=\$${prefix}/share/info $(confflags) \ + --disable-silent-rules \ $(shell DEB_CFLAGS_MAINT_APPEND=-Wall dpkg-buildflags --export=configure) cd build && $(MAKE) >$@ commit f65d8693fcb4f9cf5163a459103842adb7df6e70 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 12:38:02 2013 +0200 Bump debhelper compat to 7. diff --git a/debian/changelog b/debian/changelog index 3107171..c647840 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,7 @@ libdmx (1:1.1.3-1) UNRELEASED; urgency=low * New upstream release. * Use dpkg-buildflags. + * Bump debhelper compat to 7. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:19:24 +0200 diff --git a/debian/compat b/debian/compat index 7ed6ff8..7f8f011 100644 --- a/debian/compat +++ b/debian/compat @@ -1 +1 @@ -5 +7 commit 989fc8d611477cd4dd3af0ab1d3a93e651f08f99 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 12:35:46 2013 +0200 Use dpkg-buildflags. diff --git a/debian/changelog b/debian/changelog index 70b5b43..3107171 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,6 +1,7 @@ libdmx (1:1.1.3-1) UNRELEASED; urgency=low * New upstream release. + * Use dpkg-buildflags. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:19:24 +0200 diff --git a/debian/rules b/debian/rules index 4ac849d..3bc7584 100755 --- a/debian/rules +++ b/debian/rules @@ -10,12 +10,6 @@ # set this to the name of the main shlib's binary package PACKAGE = libdmx1 -CFLAGS = -Wall -g -ifneq (,$(filter noopt,$(DEB_BUILD_OPTIONS))) - CFLAGS += -O0 -else - CFLAGS += -O2 -endif ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) NUMJOBS = $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) MAKEFLAGS += -j$(NUMJOBS) @@ -42,15 +36,13 @@ build-stamp: ../configure --prefix=/usr --mandir=\$${prefix}/share/man \ --libdir=\$${prefix}/lib/$(DEB_HOST_MULTIARCH) \ --infodir=\$${prefix}/share/info $(confflags) \ - CFLAGS="$(CFLAGS)" + $(shell DEB_CFLAGS_MAINT_APPEND=-Wall dpkg-buildflags --export=configure) cd build && $(MAKE) - - touch build-stamp + >$@ clean: dh_testdir rm -f build-stamp - rm -f config.cache config.log config.status rm -f */config.cache */config.log */config.status rm -f conftest* */conftest* @@ -59,7 +51,6 @@ clean: rm -f INSTALL aclocal.m4 config.guess config.h.in config.sub configure rm -f depcomp install-sh ltmain.sh missing mkinstalldirs find -name Makefile.in -delete - dh_clean install: build @@ -67,14 +58,12 @@ install: build dh_testroot dh_clean -k dh_installdirs - cd build && $(MAKE) DESTDIR=$(CURDIR)/debian/tmp install # Build architecture-dependent files here. binary-arch: build install dh_testdir dh_testroot - dh_installdocs dh_install --sourcedir=debian/tmp --fail-missing -X.la dh_installchangelogs commit 97e6857dfaaa9c02b2705ae1820379c67e3f0182 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 12:31:05 2013 +0200 Bump changelogs diff --git a/ChangeLog b/ChangeLog index cdc3e13..62dd42e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,60 @@ +commit 76e841968ceb69095eb0efcd435fc47440e86d2c +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Tue May 28 16:45:02 2013 -0700 + + libdmx 1.1.3 + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 5074d9d64192bd04519a438062b7d5bf216d06ee +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +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 <ivansprun...@ioactive.com> + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit b6fe1a7af34ea620e002fc453f9c5eacf7db3969 +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +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 <ivansprun...@ioactive.com> + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 78e11efe70d00063c830475eaaaa42f19380755d +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +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 <ivansprun...@ioactive.com> + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit f34f6f64698c3b957aadba7315bb13726e3d79b0 +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +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 <alan.coopersm...@oracle.com> + commit 9f470c92bc2d194c8abb9154f42864e6c82f43ef Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Mar 7 21:43:10 2012 -0800 diff --git a/debian/changelog b/debian/changelog index c4da2d7..70b5b43 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +libdmx (1:1.1.3-1) UNRELEASED; urgency=low + + * New upstream release. + + -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 12:19:24 +0200 + libdmx (1:1.1.2-1+deb7u1) wheezy-security; urgency=high * integer overflows calculating memory needs for replies [CVE-2013-1992] commit 76e841968ceb69095eb0efcd435fc47440e86d2c Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Tue May 28 16:45:02 2013 -0700 libdmx 1.1.3 Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> 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 0df9b05bf69b1413433577d5e46c280290456c8b Author: Julien Cristau <jcris...@debian.org> Date: Wed May 15 20:13:37 2013 +0200 Upload to wheezy-security diff --git a/debian/changelog b/debian/changelog index e802838..c4da2d7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +libdmx (1:1.1.2-1+deb7u1) wheezy-security; urgency=high + + * integer overflows calculating memory needs for replies [CVE-2013-1992] + + -- Julien Cristau <jcris...@debian.org> Wed, 15 May 2013 20:12:21 +0200 + libdmx (1:1.1.2-1) unstable; urgency=low [ Robert Hooker ] commit e99aaae2ee15d977496a51d67378987aaf9cf298 Author: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> Signed-off-by: Julien Cristau <jcris...@debian.org> 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 aa72ec9eb440898789c2bcdd4446f07e416628e3 Author: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> Signed-off-by: Julien Cristau <jcris...@debian.org> 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 b03b651fda6a8e4e45c7c9515a8409727d64eb3f Author: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> Signed-off-by: Julien Cristau <jcris...@debian.org> 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 7aeea88767897d1208baeed4e6386a55e448606a Author: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <alan.coopersm...@oracle.com> Signed-off-by: Julien Cristau <jcris...@debian.org> 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 + /***************************************************************************** * * commit 5074d9d64192bd04519a438062b7d5bf216d06ee Author: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <alan.coopersm...@oracle.com> 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 <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> 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 <alan.coopersm...@oracle.com> 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 <alan.coopersm...@oracle.com> 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 debian-x-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/e1uoahd-0004gt...@vasks.debian.org