summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Coopersmith <alan.coopersmith@oracle.com>2013-03-09 13:48:28 -0800
committerAlan Coopersmith <alan.coopersmith@oracle.com>2013-05-04 16:35:55 -0700
commitb6fe1a7af34ea620e002fc453f9c5eacf7db3969 (patch)
treef8efc28f8b1ea069ede08e6ae755c3ab42c12ceb
parent78e11efe70d00063c830475eaaaa42f19380755d (diff)
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>
-rw-r--r--src/dmx.c34
1 files 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