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>

1 files 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)