Fix CVE-2022-44617: Runaway loop with width of 0 and enormous height

When reading XPM images from a file with libXpm 3.5.14 or older, if a
image has a width of 0 and a very large height, the ParsePixels() function
will loop over the entire height calling getc() and ungetc() repeatedly,
or in some circumstances, may loop seemingly forever, which may cause a
denial of service to the calling program when given a small crafted XPM
file to parse.

Closes: #2

Reported-by: Martin Ettl <ettl.martin78@googlemail.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

2 files changed, 41 insertions, 10 deletions

diff --git a/src/data.c b/src/data.c
index bfad4ff..7524e65 100644
--- a/src/data.c
+++ b/src/data.c
@@ -195,19 +195,23 @@ xpmNextString(xpmData *data)
 	register char c;
 
 	/* get to the end of the current string */
-	if (data->Eos)
-	    while ((c = *data->cptr++) && c != data->Eos);
+	if (data->Eos) {
+	    while ((c = *data->cptr++) && c != data->Eos && c != '\0');
+
+	    if (c == '\0')
+		return XpmFileInvalid;
+	}
 
 	/*
 	 * then get to the beginning of the next string looking for possible
 	 * comment
 	 */
 	if (data->Bos) {
-	    while ((c = *data->cptr++) && c != data->Bos)
+	    while ((c = *data->cptr++) && c != data->Bos && c != '\0')
 		if (data->Bcmt && c == data->Bcmt[0])
 		    ParseComment(data);
 	} else if (data->Bcmt) {	/* XPM2 natural */
-	    while ((c = *data->cptr++) == data->Bcmt[0])
+	    while (((c = *data->cptr++) == data->Bcmt[0]) && c != '\0')
 		ParseComment(data);
 	    data->cptr--;
 	}
@@ -216,9 +220,13 @@ xpmNextString(xpmData *data)
 	FILE *file = data->stream.file;
 
 	/* get to the end of the current string */
-	if (data->Eos)
+	if (data->Eos) {
 	    while ((c = Getc(data, file)) != data->Eos && c != EOF);
 
+	    if (c == EOF)
+		return XpmFileInvalid;
+	}
+
 	/*
 	 * then get to the beginning of the next string looking for possible
 	 * comment
@@ -234,7 +242,7 @@ xpmNextString(xpmData *data)
 	    Ungetc(data, c, file);
 	}
     }
-    return 0;
+    return XpmSuccess;
 }
 
 
diff --git a/src/parse.c b/src/parse.c
index 037fc66..64f51ba 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -427,6 +427,13 @@ ParsePixels(
 {
     unsigned int *iptr, *iptr2 = NULL; /* found by Egbert Eich */
     unsigned int a, x, y;
+    int ErrorStatus;
+
+    if ((width == 0) && (height != 0))
+	return (XpmFileInvalid);
+
+    if ((height == 0) && (width != 0))
+	return (XpmFileInvalid);
 
     if ((height > 0 && width >= UINT_MAX / height) ||
 	width * height >= UINT_MAX / sizeof(unsigned int))
@@ -464,7 +471,11 @@ ParsePixels(
 		colidx[(unsigned char)colorTable[a].string[0]] = a + 1;
 
 	    for (y = 0; y < height; y++) {
-		xpmNextString(data);
+		ErrorStatus = xpmNextString(data);
+		if (ErrorStatus != XpmSuccess) {
+		    XpmFree(iptr2);
+		    return (ErrorStatus);
+		}
 		for (x = 0; x < width; x++, iptr++) {
 		    int c = xpmGetC(data);
 
@@ -511,7 +522,11 @@ do \
 	    }
 
 	    for (y = 0; y < height; y++) {
-		xpmNextString(data);
+		ErrorStatus = xpmNextString(data);
+		if (ErrorStatus != XpmSuccess) {
+		    XpmFree(iptr2);
+		    return (ErrorStatus);
+		}
 		for (x = 0; x < width; x++, iptr++) {
 		    int cc1 = xpmGetC(data);
 		    if (cc1 > 0 && cc1 < 256) {
@@ -551,7 +566,11 @@ do \
 		xpmHashAtom *slot;
 
 		for (y = 0; y < height; y++) {
-		    xpmNextString(data);
+		    ErrorStatus = xpmNextString(data);
+		    if (ErrorStatus != XpmSuccess) {
+			XpmFree(iptr2);
+			return (ErrorStatus);
+		    }
 		    for (x = 0; x < width; x++, iptr++) {
 			for (a = 0, s = buf; a < cpp; a++, s++) {
 			    int c = xpmGetC(data);
@@ -571,7 +590,11 @@ do \
 		}
 	    } else {
 		for (y = 0; y < height; y++) {
-		    xpmNextString(data);
+		    ErrorStatus = xpmNextString(data);
+		    if (ErrorStatus != XpmSuccess) {
+			XpmFree(iptr2);
+			return (ErrorStatus);
+		    }
 		    for (x = 0; x < width; x++, iptr++) {
 			for (a = 0, s = buf; a < cpp; a++, s++) {
 			    int c = xpmGetC(data);