Integer overflows in stringSectionSize() cause buffer overflow in…

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>

Integer overflows in stringSectionSize() cause buffer overflow in…
0349af11 · Alan Coopersmith · Ulrich Sibiller · 8673bf07 · 0349af11
Commit 0349af11 authored Mar 02, 2013 by Alan Coopersmith Committed by Ulrich Sibiller Oct 12, 2016
Show whitespace changes
Inline Side-by-side

Showing with 21 additions and 6 deletions

cmsColNm.c nx-X11/lib/X11/cmsColNm.c +21 -6

No files found.
--- 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);
 }