libdmx: Changes to 'debian-unstable'
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 <jcristau@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 <jcristau@debian.org> Sun, 16 Jun 2013 12:19:24 +0200
+ -- Julien Cristau <jcristau@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 <jcristau@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 <jcristau@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 <jcristau@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 <jcristau@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 <jcristau@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 <jcristau@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 <jcristau@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.coopersmith@oracle.com>
+Date: Tue May 28 16:45:02 2013 -0700
+
+ libdmx 1.1.3
+
+ Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
+
+commit 5074d9d64192bd04519a438062b7d5bf216d06ee
+Author: Alan Coopersmith <alan.coopersmith@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 <ivansprundel@ioactive.com>
+ Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
+
+commit b6fe1a7af34ea620e002fc453f9c5eacf7db3969
+Author: Alan Coopersmith <alan.coopersmith@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 <ivansprundel@ioactive.com>
+ Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
+
+commit 78e11efe70d00063c830475eaaaa42f19380755d
+Author: Alan Coopersmith <alan.coopersmith@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 <ivansprundel@ioactive.com>
+ Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
+
+commit f34f6f64698c3b957aadba7315bb13726e3d79b0
+Author: Alan Coopersmith <alan.coopersmith@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.coopersmith@oracle.com>
+
commit 9f470c92bc2d194c8abb9154f42864e6c82f43ef
Author: Alan Coopersmith <alan.coopersmith@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 <jcristau@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.coopersmith@oracle.com>
Date: Tue May 28 16:45:02 2013 -0700
libdmx 1.1.3
Signed-off-by: Alan Coopersmith <alan.coopersmith@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 <jcristau@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 <jcristau@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Julien Cristau <jcristau@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Julien Cristau <jcristau@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Julien Cristau <jcristau@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.coopersmith@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.coopersmith@oracle.com>
Signed-off-by: Julien Cristau <jcristau@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@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.coopersmith@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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@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.coopersmith@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.coopersmith@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
+
/*****************************************************************************
* *
Reply to: