From 99c644fc8488657bdd106717df7446d606f9ef22 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 8 Mar 2013 19:55:55 -0800 Subject: integer overflow in XineramaQueryScreens() [CVE-2013-1985] If the reported number of screens is too large, the calculations to allocate memory for them may overflow, leaving us writing beyond the bounds of the allocation. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith --- src/Xinerama.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/Xinerama.c b/src/Xinerama.c index 04189b6..67a35b5 100644 --- a/src/Xinerama.c +++ b/src/Xinerama.c @@ -303,24 +303,36 @@ XineramaQueryScreens( return NULL; } - if(rep.number) { - if((scrnInfo = Xmalloc(sizeof(XineramaScreenInfo) * rep.number))) { + /* + * rep.number 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, Xorg is currently limited + * to 16 screens, and few known servers have a much higher limit, + * so 1024 seems more than enough to prevent both integer overflow + * and insane X server responses causing massive memory allocation. + */ + if ((rep.number > 0) && (rep.number <= 1024)) + scrnInfo = Xmalloc(sizeof(XineramaScreenInfo) * rep.number); + if (scrnInfo != NULL) { + int i; + + for (i = 0; i < rep.number; i++) { xXineramaScreenInfo scratch; - int i; - - for(i = 0; i < rep.number; i++) { - _XRead(dpy, (char*)(&scratch), sz_XineramaScreenInfo); - scrnInfo[i].screen_number = i; - scrnInfo[i].x_org = scratch.x_org; - scrnInfo[i].y_org = scratch.y_org; - scrnInfo[i].width = scratch.width; - scrnInfo[i].height = scratch.height; - } - - *number = rep.number; - } else - _XEatDataWords(dpy, rep.length); + + _XRead(dpy, (char*)(&scratch), sz_XineramaScreenInfo); + + scrnInfo[i].screen_number = i; + scrnInfo[i].x_org = scratch.x_org; + scrnInfo[i].y_org = scratch.y_org; + scrnInfo[i].width = scratch.width; + scrnInfo[i].height = scratch.height; + } + + *number = rep.number; } else { + _XEatDataWords(dpy, rep.length); *number = 0; } -- cgit v1.2.1