From: Tobias Stoeckmann <[email protected]>

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

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to