Avoid OOB write when handling malicious XPM files.

libXpm uses unsigned int to store sizes, which fits size_t on 32 bit
systems, but leads to issues on 64 bit systems.

On 64 bit systems, it is possible to overflow 32 bit integers while
parsing XPM extensions in a file.

At first, it looks like a rather unimportant detail, because nobody
will seriously open a 4 GB file. But unfortunately XPM has support for
gzip compression out of the box. An attacker can therefore craft a
compressed file which is merely 4 MB in size, which makes an attack
much for feasable.

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

1 files changed, 25 insertions, 9 deletions

diff --git a/src/CrDatFrI.c b/src/CrDatFrI.c
index 0dacf51..6735bfc 100644
--- a/src/CrDatFrI.c
+++ b/src/CrDatFrI.c
@@ -48,7 +48,7 @@ LFUNC(CreatePixels, void, (char **dataptr, unsigned int data_size,
 			   unsigned int height, unsigned int cpp,
 			   unsigned int *pixels, XpmColor *colors));
 
-LFUNC(CountExtensions, void, (XpmExtension *ext, unsigned int num,
+LFUNC(CountExtensions, int, (XpmExtension *ext, unsigned int num,
 			      unsigned int *ext_size,
 			      unsigned int *ext_nlines));
 
@@ -122,8 +122,9 @@ XpmCreateDataFromXpmImage(
 
     /* compute the number of extensions lines and size */
     if (extensions)
-	CountExtensions(info->extensions, info->nextensions,
-			&ext_size, &ext_nlines);
+	if (CountExtensions(info->extensions, info->nextensions,
+			&ext_size, &ext_nlines))
+	    return(XpmNoMemory);
 
     /*
      * alloc a temporary array of char pointer for the header section which
@@ -187,7 +188,8 @@ XpmCreateDataFromXpmImage(
     if(offset <= image->width || offset <= image->cpp)
 	RETURN(XpmNoMemory);
 
-    if( (image->height + ext_nlines) >= UINT_MAX / sizeof(char *))
+    if (image->height > UINT_MAX - ext_nlines ||
+	image->height + ext_nlines >= UINT_MAX / sizeof(char *))
 	RETURN(XpmNoMemory);
     data_size = (image->height + ext_nlines) * sizeof(char *);
 
@@ -196,7 +198,8 @@ XpmCreateDataFromXpmImage(
 	RETURN(XpmNoMemory);
     data_size += image->height * offset;
 
-    if( (header_size + ext_size) >= (UINT_MAX - data_size) )
+    if (header_size > UINT_MAX - ext_size ||
+	header_size + ext_size >= (UINT_MAX - data_size) )
 	RETURN(XpmNoMemory);
     data_size += header_size + ext_size;
 
@@ -343,13 +346,14 @@ CreatePixels(
     *s = '\0';
 }
 
-static void
+static int
 CountExtensions(
     XpmExtension	*ext,
     unsigned int	 num,
     unsigned int	*ext_size,
     unsigned int	*ext_nlines)
 {
+    size_t len;
     unsigned int x, y, a, size, nlines;
     char **line;
 
@@ -357,16 +361,28 @@ CountExtensions(
     nlines = 0;
     for (x = 0; x < num; x++, ext++) {
 	/* 1 for the name */
+	if (ext->nlines == UINT_MAX || nlines > UINT_MAX - ext->nlines - 1)
+	    return (1);
 	nlines += ext->nlines + 1;
 	/* 8 = 7 (for "XPMEXT ") + 1 (for 0) */
-	size += strlen(ext->name) + 8;
+	len = strlen(ext->name) + 8;
+	if (len > UINT_MAX - size)
+	    return (1);
+	size += len;
 	a = ext->nlines;
-	for (y = 0, line = ext->lines; y < a; y++, line++)
-	    size += strlen(*line) + 1;
+	for (y = 0, line = ext->lines; y < a; y++, line++) {
+	    len = strlen(*line) + 1;
+	    if (len > UINT_MAX - size)
+		return (1);
+	    size += len;
+	}
     }
+    if (size > UINT_MAX - 10 || nlines > UINT_MAX - 1)
+	return (1);
     /* 10 and 1 are for the ending "XPMENDEXT" */
     *ext_size = size + 10;
     *ext_nlines = nlines + 1;
+    return (0);
 }
 
 static void