libxpm: Changes to 'upstream-unstable'
configure.ac | 2 +-
src/CrDatFrI.c | 34 +++++++++++++++++++++++++---------
src/RdFToBuf.c | 4 ++++
src/WrFFrBuf.c | 2 +-
src/create.c | 11 ++++++-----
src/parse.c | 40 ++++++++++++++++++++++++++++++++--------
6 files changed, 69 insertions(+), 24 deletions(-)
New commits:
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>
diff --git a/configure.ac b/configure.ac
index 46e2a27..2feb9ff 100644
--- a/configure.ac
+++ b/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])
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>
diff --git a/src/RdFToBuf.c b/src/RdFToBuf.c
index 7f8ebee..69e3347 100644
--- a/src/RdFToBuf.c
+++ b/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/src/WrFFrBuf.c b/src/WrFFrBuf.c
index b80aa62..0e57cc8 100644
--- a/src/WrFFrBuf.c
+++ b/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)
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>
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
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>
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;
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>
diff --git a/src/create.c b/src/create.c
index d013da9..a750846 100644
--- a/src/create.c
+++ b/src/create.c
@@ -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;
}
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>
diff --git a/src/create.c b/src/create.c
index 98678d8..d013da9 100644
--- a/src/create.c
+++ b/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 +
Reply to: