[PATCH libXpm] Avoid OOB write when handling malicious XPM files.
Matthieu Herrb
matthieu at herrb.eu
Thu Dec 8 16:07:55 UTC 2016
From: Tobias Stoeckmann <tobias at stoeckmann.org>
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.
---
src/CrDatFrI.c | 34 +++++++++++++++++++++++++---------
1 file 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
--
1.9.1
More information about the xorg-devel
mailing list