[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

libdmx: Changes to 'upstream-unstable'



 configure.ac |    9 +++++-
 src/dmx.c    |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 82 insertions(+), 11 deletions(-)

New commits:
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 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: