Gracefully handle EOF while parsing files.

libXpm does not properly handle EOF conditions when xpmGetC is called
multiple times in a row to construct a string. Instead of checking
its return value for EOF, the result is automatically casted into a
char and attached to a string.

By carefully crafting the color table in an XPM file, it is possible to
send a libXpm program like gimp into a very long lasting loop and
massive memory allocations.

Otherwise no memory issues arise, therefore this is just a purely
functional patch to dismiss invalid input.

Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <Matthieu@herrb.eu>

1 files changed, 32 insertions, 8 deletions

diff --git a/src/parse.c b/src/parse.c
index ff23a47..c19209c 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -234,8 +234,14 @@ xpmParseColors(
 		xpmFreeColorTable(colorTable, ncolors);
 		return (XpmNoMemory);
 	    }
-	    for (b = 0, s = color->string; b < cpp; b++, s++)
-		*s = xpmGetC(data);
+	    for (b = 0, s = color->string; b < cpp; b++, s++) {
+		int c = xpmGetC(data);
+		if (c < 0) {
+		    xpmFreeColorTable(colorTable, ncolors);
+		    return (XpmFileInvalid);
+		}
+		*s = (char) c;
+	    }
 	    *s = '\0';
 
 	    /*
@@ -322,8 +328,14 @@ xpmParseColors(
 		xpmFreeColorTable(colorTable, ncolors);
 		return (XpmNoMemory);
 	    }
-	    for (b = 0, s = color->string; b < cpp; b++, s++)
-		*s = xpmGetC(data);
+	    for (b = 0, s = color->string; b < cpp; b++, s++) {
+		int c = xpmGetC(data);
+		if (c < 0) {
+		    xpmFreeColorTable(colorTable, ncolors);
+		    return (XpmFileInvalid);
+		}
+		*s = (char) c;
+	    }
 	    *s = '\0';
 
 	    /*
@@ -505,8 +517,14 @@ do \
 		for (y = 0; y < height; y++) {
 		    xpmNextString(data);
 		    for (x = 0; x < width; x++, iptr++) {
-			for (a = 0, s = buf; a < cpp; a++, s++)
-			    *s = xpmGetC(data); /* int assigned to char, not a problem here */
+			for (a = 0, s = buf; a < cpp; a++, s++) {
+			    int c = xpmGetC(data);
+			    if (c < 0) {
+				XpmFree(iptr2);
+				return (XpmFileInvalid);
+			    }
+			    *s = (char) c;
+			}
 			slot = xpmHashSlot(hashtable, buf);
 			if (!*slot) {	/* no color matches */
 			    XpmFree(iptr2);
@@ -519,8 +537,14 @@ do \
 		for (y = 0; y < height; y++) {
 		    xpmNextString(data);
 		    for (x = 0; x < width; x++, iptr++) {
-			for (a = 0, s = buf; a < cpp; a++, s++)
-			    *s = xpmGetC(data); /* int assigned to char, not a problem here */
+			for (a = 0, s = buf; a < cpp; a++, s++) {
+			    int c = xpmGetC(data);
+			    if (c < 0) {
+				XpmFree(iptr2);
+				return (XpmFileInvalid);
+			    }
+			    *s = (char) c;
+			}
 			for (a = 0; a < ncolors; a++)
 			    if (!strcmp(colorTable[a].string, buf))
 				break;