integer overflow in XListFontsWithInfo() [CVE-2013-1981 3/13]

If the reported number of remaining fonts is too large, the calculations
to allocate memory for them may overflow, leaving us writing beyond the
bounds of the allocation.

v2: Fix reply_left calculations, check calculated sizes fit in reply_left
v3: On error cases, also set values to be returned in pointer args to 0/NULL

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

1 files changed, 49 insertions, 56 deletions

diff --git a/src/FontInfo.c b/src/FontInfo.c
index fb296b8a..4b295a5e 100644
--- a/src/FontInfo.c
+++ b/src/FontInfo.c
@@ -28,6 +28,7 @@ in this Software without prior written authorization from The Open Group.
 #include <config.h>
 #endif
 #include "Xlibint.h"
+#include <limits.h>
 
 #if defined(XF86BIGFONT)
 #define USE_XF86BIGFONT
@@ -45,10 +46,11 @@ int maxNames,
 int *actualCount,	/* RETURN */
 XFontStruct **info)	/* RETURN */
 {
-    register long nbytes;
+    unsigned long nbytes;
+    unsigned long reply_left;	/* unused data left in reply buffer */
     register int i;
     register XFontStruct *fs;
-    register int size = 0;
+    unsigned int size = 0;
     XFontStruct *finfo = NULL;
     char **flist = NULL;
     xListFontsWithInfoReply reply;
@@ -67,52 +69,44 @@ XFontStruct **info)	/* RETURN */
 	if (!_XReply (dpy, (xReply *) &reply,
 		      ((SIZEOF(xListFontsWithInfoReply) -
 			SIZEOF(xGenericReply)) >> 2), xFalse)) {
-	    for (j=(i-1); (j >= 0); j--) {
-		Xfree(flist[j]);
-		if (finfo[j].properties) Xfree((char *) finfo[j].properties);
-	    }
-	    if (flist) Xfree((char *) flist);
-	    if (finfo) Xfree((char *) finfo);
-	    UnlockDisplay(dpy);
-	    SyncHandle();
-	    return ((char **) NULL);
+	    reply.nameLength = 0; /* avoid trying to read more replies */
+	    reply_left = 0;
+	    goto badmem;
 	}
-	if (reply.nameLength == 0)
+	reply_left = reply.length -
+	    ((SIZEOF(xListFontsWithInfoReply) -	SIZEOF(xGenericReply)) >> 2);
+	if (reply.nameLength == 0) {
+	    _XEatDataWords(dpy, reply_left);
 	    break;
+	}
+	if (reply.nReplies >= (INT_MAX - i)) /* avoid overflowing size */
+	    goto badmem;
 	if ((i + reply.nReplies) >= size) {
 	    size = i + reply.nReplies + 1;
 
+	    if (size >= (INT_MAX / sizeof(XFontStruct)))
+		goto badmem;
+
 	    if (finfo) {
-		XFontStruct * tmp_finfo = (XFontStruct *)
-		    Xrealloc ((char *) finfo,
-			      (unsigned) (sizeof(XFontStruct) * size));
-		char ** tmp_flist = (char **)
-		    Xrealloc ((char *) flist,
-			      (unsigned) (sizeof(char *) * (size+1)));
+		XFontStruct * tmp_finfo;
+		char ** tmp_flist;
 
+		tmp_finfo = Xrealloc (finfo, sizeof(XFontStruct) * size);
 		if (tmp_finfo)
 		    finfo = tmp_finfo;
+		else
+		    goto badmem;
+
+		tmp_flist = Xrealloc (flist, sizeof(char *) * (size+1));
 		if (tmp_flist)
 		    flist = tmp_flist;
-
-		if ((! tmp_finfo) || (! tmp_flist)) {
-		    /* free all the memory that we allocated */
-		    for (j=(i-1); (j >= 0); j--) {
-			Xfree(flist[j]);
-			if (finfo[j].properties)
-			    Xfree((char *) finfo[j].properties);
-		    }
-		    Xfree((char *) flist);
-		    Xfree((char *) finfo);
-		    goto clearwire;
-		}
+		else
+		    goto badmem;
 	    }
 	    else {
-		if (! (finfo = (XFontStruct *)
-		       Xmalloc((unsigned) (sizeof(XFontStruct) * size))))
+		if (! (finfo = Xmalloc(sizeof(XFontStruct) * size)))
 		    goto clearwire;
-		if (! (flist = (char **)
-		       Xmalloc((unsigned) (sizeof(char *) * (size+1))))) {
+		if (! (flist = Xmalloc(sizeof(char *) * (size+1)))) {
 		    Xfree((char *) finfo);
 		    goto clearwire;
 		}
@@ -138,24 +132,27 @@ XFontStruct **info)	/* RETURN */
 	fs->max_bounds = * (XCharStruct *) &reply.maxBounds;
 
 	fs->n_properties = reply.nFontProps;
+	fs->properties = NULL;
 	if (fs->n_properties > 0) {
-	    nbytes = reply.nFontProps * sizeof(XFontProp);
-	    if (! (fs->properties = (XFontProp *) Xmalloc((unsigned) nbytes)))
-		goto badmem;
+	    /* nFontProps is a CARD16 */
 	    nbytes = reply.nFontProps * SIZEOF(xFontProp);
+	    if ((nbytes >> 2) <= reply_left) {
+		size_t pbytes = reply.nFontProps * sizeof(XFontProp);
+		fs->properties = Xmalloc (pbytes);
+	    }
+	    if (! fs->properties)
+		goto badmem;
 	    _XRead32 (dpy, (long *)fs->properties, nbytes);
+	    reply_left -= (nbytes >> 2);
+	}
 
-	} else
-	    fs->properties = NULL;
-
-	j = reply.nameLength + 1;
+	/* nameLength is a CARD8 */
+	nbytes = reply.nameLength + 1;
 	if (!i)
-	    j++; /* make first string 1 byte longer, to match XListFonts */
-	flist[i] = (char *) Xmalloc ((unsigned int) j);
+	    nbytes++; /* make first string 1 byte longer, to match XListFonts */
+	flist[i] = Xmalloc (nbytes);
 	if (! flist[i]) {
 	    if (finfo[i].properties) Xfree((char *) finfo[i].properties);
-	    nbytes = (reply.nameLength + 3) & ~3;
-	    _XEatData(dpy, (unsigned long) nbytes);
 	    goto badmem;
 	}
 	if (!i) {
@@ -185,19 +182,15 @@ XFontStruct **info)	/* RETURN */
 
   clearwire:
     /* Clear the wire. */
-    do {
-	if (reply.nFontProps)
-	    _XEatData(dpy, (unsigned long)
-		      (reply.nFontProps * SIZEOF(xFontProp)));
-	nbytes = (reply.nameLength + 3) & ~3;
-	_XEatData(dpy, (unsigned long) nbytes);
-    }
-    while (_XReply(dpy,(xReply *) &reply, ((SIZEOF(xListFontsWithInfoReply) -
-					    SIZEOF(xGenericReply)) >> 2),
-		   xFalse) && (reply.nameLength != 0));
-
+    _XEatDataWords(dpy, reply_left);
+    while ((reply.nameLength != 0) &&
+	   _XReply(dpy, (xReply *) &reply,
+		   ((SIZEOF(xListFontsWithInfoReply) - SIZEOF(xGenericReply))
+		    >> 2), xTrue));
     UnlockDisplay(dpy);
     SyncHandle();
+    *info = NULL;
+    *actualCount = 0;
     return (char **) NULL;
 }