Update to libXpm 3.5.12

8 files changed, 168 insertions, 37 deletions

diff --git a/lib/libXpm/ChangeLog b/lib/libXpm/ChangeLog
index f6caf6116..d66458094 100644
--- a/lib/libXpm/ChangeLog
+++ b/lib/libXpm/ChangeLog
@@ -1,3 +1,92 @@
+commit 1fab5e81fd761f628fb68d22934615536dbd0220
+Author: Matthieu Herrb <matthieu@herrb.eu>
+Date:   Mon Dec 12 23:09:52 2016 +0100
+
+    libXpm 3.5.12
+    
+    Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
+
+commit 8b3024e6871ce50b34bf2dff924774bd654703bc
+Author: Tobias Stoeckmann <tobias@stoeckmann.org>
+Date:   Sun Dec 11 13:50:05 2016 +0100
+
+    Handle size_t in file/buffer length
+    
+    The values of file sizes and buffer sizes can exceed current limits.
+    Therefore, use proper variable types for these operations.
+    
+    Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
+    Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
+
+commit d1167418f0fd02a27f617ec5afd6db053afbe185
+Author: Tobias Stoeckmann <tobias@stoeckmann.org>
+Date:   Thu Dec 8 17:07:55 2016 +0100
+
+    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>
+
+commit 1ec33006a9e4214b390045b820464e24297dc6c0
+Author: Tobias Stoeckmann <tobias@stoeckmann.org>
+Date:   Tue Dec 6 22:34:33 2016 +0100
+
+    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>
+
+commit c46dedeba15edf7216d62633ed6daf40cd1f5bfd
+Author: Tobias Stoeckmann <tobias@stoeckmann.org>
+Date:   Tue Dec 6 22:31:53 2016 +0100
+
+    Fix out out boundary read on unknown colors
+    
+    libXpm is vulnerable to an out of boundary read if an XPM file contains
+    a color with a symbolic name but without any default color value.
+    
+    A caller must set XpmColorSymbols and a color with a NULL name in
+    the supplied XpmAttributes to XpmReadFileToImage (or other functions of
+    this type) in order to trigger this issue.
+    
+    Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
+    Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
+
+commit 42ca8d956276bc00bec09e410d76daf053ae35f9
+Author: Jörg Sonnenberger <joerg@NetBSD.org>
+Date:   Wed Mar 19 09:26:37 2014 +0100
+
+    Fix abs() usage.
+    
+    For long arguments, use labs().
+    
+    Reviewed-by: Matt Turner <mattst88@gmail.com>
+    Signed-off-by: Thomas Klausner <wiz@NetBSD.org>
+
 commit 3425cbb0e6086f74783eafbe23df1121b655e006
 Author: Alan Coopersmith <alan.coopersmith@oracle.com>
 Date:   Sat Sep 7 21:40:17 2013 -0700
@@ -970,9 +1059,6 @@ Date:   Tue Sep 21 17:57:35 2004 +0000
     Removed inclusion of unnecessary kernel header on Linux. This may fail in
         an -ansi environment.
 
-Notes:
-    Fixes CVE-2004-0687 (integer overflows) and CVE-2004-0688 (stack overflows)
-
 commit 2773a7214e282f6f673483f5233b880505947c3f
 Author: Egbert Eich <eich@suse.de>
 Date:   Fri Apr 23 18:42:32 2004 +0000
diff --git a/lib/libXpm/configure b/lib/libXpm/configure
index 92dcd22eb..7a53c8b2b 100644
--- a/lib/libXpm/configure
+++ b/lib/libXpm/configure
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for libXpm 3.5.11.
+# Generated by GNU Autoconf 2.69 for libXpm 3.5.12.
 #
 # Report bugs to <https://bugs.freedesktop.org/enter_bug.cgi?product=xorg>.
 #
@@ -591,8 +591,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='libXpm'
 PACKAGE_TARNAME='libXpm'
-PACKAGE_VERSION='3.5.11'
-PACKAGE_STRING='libXpm 3.5.11'
+PACKAGE_VERSION='3.5.12'
+PACKAGE_STRING='libXpm 3.5.12'
 PACKAGE_BUGREPORT='https://bugs.freedesktop.org/enter_bug.cgi?product=xorg'
 PACKAGE_URL=''
 
@@ -1357,7 +1357,7 @@ if test "$ac_init_help" = "long"; then
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures libXpm 3.5.11 to adapt to many kinds of systems.
+\`configure' configures libXpm 3.5.12 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1427,7 +1427,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of libXpm 3.5.11:";;
+     short | recursive ) echo "Configuration of libXpm 3.5.12:";;
    esac
   cat <<\_ACEOF
 
@@ -1554,7 +1554,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-libXpm configure 3.5.11
+libXpm configure 3.5.12
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -1878,7 +1878,7 @@ cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by libXpm $as_me 3.5.11, which was
+It was created by libXpm $as_me 3.5.12, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -2708,7 +2708,7 @@ fi
 
 # Define the identity of the package.
  PACKAGE='libXpm'
- VERSION='3.5.11'
+ VERSION='3.5.12'
 
 
 cat >>confdefs.h <<_ACEOF
@@ -18408,7 +18408,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by libXpm $as_me 3.5.11, which was
+This file was extended by libXpm $as_me 3.5.12, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -18474,7 +18474,7 @@ _ACEOF
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-libXpm config.status 3.5.11
+libXpm config.status 3.5.12
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 
diff --git a/lib/libXpm/configure.ac b/lib/libXpm/configure.ac
index 46e2a2776..2feb9ffe1 100644
--- a/lib/libXpm/configure.ac
+++ b/lib/libXpm/configure.ac
@@ -1,7 +1,7 @@
 
 # Initialize Autoconf
 AC_PREREQ([2.60])
-AC_INIT([libXpm], [3.5.11],
+AC_INIT([libXpm], [3.5.12],
         [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg], [libXpm])
 AC_CONFIG_SRCDIR([Makefile.am])
 AC_CONFIG_HEADERS([config.h])
diff --git a/lib/libXpm/src/CrDatFrI.c b/lib/libXpm/src/CrDatFrI.c
index 0dacf5127..6735bfcc0 100644
--- a/lib/libXpm/src/CrDatFrI.c
+++ b/lib/libXpm/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
diff --git a/lib/libXpm/src/RdFToBuf.c b/lib/libXpm/src/RdFToBuf.c
index 7f8ebee61..69e3347d2 100644
--- a/lib/libXpm/src/RdFToBuf.c
+++ b/lib/libXpm/src/RdFToBuf.c
@@ -89,6 +89,10 @@ XpmReadFileToBuffer(
 	return XpmOpenFailed;
     }
     len = stats.st_size;
+    if (len < 0 || len >= SIZE_MAX) {
+	close(fd);
+	return XpmOpenFailed;
+    }
     ptr = (char *) XpmMalloc(len + 1);
     if (!ptr) {
 	fclose(fp);
diff --git a/lib/libXpm/src/WrFFrBuf.c b/lib/libXpm/src/WrFFrBuf.c
index b80aa62ec..0e57cc886 100644
--- a/lib/libXpm/src/WrFFrBuf.c
+++ b/lib/libXpm/src/WrFFrBuf.c
@@ -44,7 +44,7 @@ XpmWriteFileFromBuffer(
     const char	*filename,
     char	*buffer)
 {
-    int fcheck, len;
+    size_t fcheck, len;
     FILE *fp = fopen(filename, "w");
 
     if (!fp)
diff --git a/lib/libXpm/src/create.c b/lib/libXpm/src/create.c
index 98678d8f3..a750846bf 100644
--- a/lib/libXpm/src/create.c
+++ b/lib/libXpm/src/create.c
@@ -347,10 +347,10 @@ SetCloseColor(
 
 	    closenesses[i].cols_index = i;
 	    closenesses[i].closeness =
-		COLOR_FACTOR * (abs((long) col->red - (long) cols[i].red)
-				+ abs((long) col->green - (long) cols[i].green)
-				+ abs((long) col->blue - (long) cols[i].blue))
-		+ BRIGHTNESS_FACTOR * abs(((long) col->red +
+		COLOR_FACTOR * (labs((long) col->red - (long) cols[i].red)
+				+ labs((long) col->green - (long) cols[i].green)
+				+ labs((long) col->blue - (long) cols[i].blue))
+		+ BRIGHTNESS_FACTOR * labs(((long) col->red +
 					   (long) col->green +
 					   (long) col->blue)
 					   - ((long) cols[i].red +
@@ -647,7 +647,8 @@ CreateColors(
 			while (def_index <= 5 && defaults[def_index] == NULL)
 			    ++def_index;
 		    }
-		    if (def_index >= 2 && defaults[def_index] != NULL &&
+		    if (def_index >= 2 && def_index <= 5 &&
+			defaults[def_index] != NULL &&
 			!xpmstrcasecmp(symbol->value, defaults[def_index]))
 			break;
 		}
diff --git a/lib/libXpm/src/parse.c b/lib/libXpm/src/parse.c
index ff23a4738..c19209cd7 100644
--- a/lib/libXpm/src/parse.c
+++ b/lib/libXpm/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;