Integer overflows in stringSectionSize() cause buffer overflow in ReadColornameDB() [CVE-2013-1981 6/13]

LoadColornameDB() calls stringSectionSize() to do a first pass over the
file (which may be provided by the user via XCMSDB environment variable)
to determine how much memory needs to be allocated to read in the file,
then allocates the returned sizes and calls ReadColornameDB() to load the
data from the file into that newly allocated memory.

If stringSectionSize() overflows the signed ints used to calculate the
file size (say if you have an xcmsdb with ~4 billion lines in or a
combined string length of ~4 gig - which while it may have been
inconceivable when Xlib was written, is quite possible today), then
LoadColornameDB() may allocate a memory buffer much smaller than the
amount of data ReadColornameDB() will write to it.

The total size is left limited to an int, because if your xcmsdb file
is larger than 2gb, you're doing it wrong.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Signed-off-by: Julien Cristau <jcristau@debian.org>
Backported-to-NX-by: Ulrich Sibiller <uli42@gmx.de>

1 files changed, 21 insertions, 6 deletions

diff --git a/nx-X11/lib/X11/cmsColNm.c b/nx-X11/lib/X11/cmsColNm.c
index 82ffab5f6..5f9724822 100644
--- a/nx-X11/lib/X11/cmsColNm.c
+++ b/nx-X11/lib/X11/cmsColNm.c
@@ -40,6 +40,7 @@
 #include <sys/stat.h>
 #include <stdio.h>
 #include <ctype.h>
+#include <limits.h>
 #define XK_LATIN1
 #include <nx-X11/keysymdef.h>
 #include "Cv.h"
@@ -542,7 +543,10 @@ stringSectionSize(
     char *pBuf;
     char *f1;
     char *f2;
-    int i;
+    size_t i;
+
+    unsigned int numEntries = 0;
+    unsigned int sectionSize = 0;
 
     *pNumEntries = 0;
     *pSectionSize = 0;
@@ -576,26 +580,37 @@ stringSectionSize(
 	    return(XcmsFailure);
 	}
 
-	(*pNumEntries)++;
+	numEntries++;
+	if (numEntries >= INT_MAX)
+	    return(XcmsFailure);
 
-	(*pSectionSize) += (i = strlen(f1)) + 1;
+	i = strlen(f1);
+	if (i >= INT_MAX - sectionSize)
+	    return(XcmsFailure);
+	sectionSize += i + 1;
 	for (; i; i--, f1++) {
 	    /* REMOVE SPACES FROM COUNT */
 	    if (isspace(*f1)) {
-		(*pSectionSize)--;
+		sectionSize--;
 	    }
 	}
 
-	(*pSectionSize) += (i = strlen(f2)) + 1;
+	i = strlen(f2);
+	if (i >= INT_MAX - sectionSize)
+	    return(XcmsFailure);
+	sectionSize += i + 1;
 	for (; i; i--, f2++) {
 	    /* REMOVE SPACES FROM COUNT */
 	    if (isspace(*f2)) {
-		(*pSectionSize)--;
+		sectionSize--;
 	    }
 	}
 
     }
 
+    *pNumEntries = (int) numEntries;
+    *pSectionSize = (int) sectionSize;
+
     return(XcmsSuccess);
 }