From f34f6f64698c3b957aadba7315bb13726e3d79b0 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Fri, 3 May 2013 23:10:47 -0700
Subject: [PATCH:libdmx 1/4] 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]>
---
configure.ac | 7 +++++++
src/dmx.c | 17 +++++++++++++++++
2 files changed, 24 insertions(+)
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
+
/*****************************************************************************
* *
--
1.7.9.2
From 78e11efe70d00063c830475eaaaa42f19380755d Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 9 Mar 2013 13:48:28 -0800
Subject: [PATCH:libdmx 2/4] 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]>
---
src/dmx.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
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)
--
1.7.9.2
From b6fe1a7af34ea620e002fc453f9c5eacf7db3969 Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 9 Mar 2013 13:48:28 -0800
Subject: [PATCH:libdmx 3/4] 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]>
---
src/dmx.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
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
--
1.7.9.2
From 5074d9d64192bd04519a438062b7d5bf216d06ee Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <[email protected]>
Date: Sat, 9 Mar 2013 13:48:28 -0800
Subject: [PATCH:libdmx 4/4] 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]>
---
src/dmx.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
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. */
--
1.7.9.2