CESA-2004-003 - rev 2

http://scary.beasts.org/security/CESA-2004-003.txt

libXpm multiple image parsing flaws
===================================

Programs affected: libXpm, and any programs which use libXpm to decode XPM
files. For example, the GIMP seems to use libXpm.
Severity: Compromise of account used to browse malicious XPM file.
CAN identifier(s): CAN-2004-0782 and CAN-2004-0783
Fixed: X.ORG release 6.8.1 contains fixes.

This advisory lists code flaws discovered by inspection of the libXpm code.
The specific version of libXpm discussed is the release that comes with the
initial X.ORG X11 system source code release. However, these flaws seem to
exist in older versions.


Flaw 1. Stack-based overflow in xpmParseColors (parse.c).
This is CAN-2004-0782

Careless use of strcat() in both the XPMv1 and XPMv2/3 parsing code leads to
a stack based overflow that should be exploitable. There are minor
complications due to stack layout; the buffer being overflowed in fact
typically overflows into another buffer that is used to populate the overflowed
buffer. This should not prevent exploitation, however.
Demo XPM: http://scary.beasts.org/misc/doom.xpm


Flaw 2. Integer overflow allocating colorTable in xpmParseColors (parse.c) -
probably a crashable but not exploitable offence. Here:
This is CAN-2004-0783

    colorTable = (XpmColor *) XpmCalloc(ncolors, sizeof(XpmColor));

ncolors would seem to come from the (untrusted) XPM file.
In fact, multiple integer overflow problems seem to exist. Some may well be
exploitable. Note that the following may not be an exhaustive list:
a) XpmCreateImageFromXpmImage: multiple possible overflow, e.g.:
    image_pixels = (Pixel *) XpmMalloc(sizeof(Pixel) * image->ncolors);
(ncolors is user-supplied)
b) CreateXImage:
    *image_return = XCreateImage(display, visual, depth, format, 0, 0,
         width, height, bitmap_pad, 0);
(width and height are user-supplied, possibly other variables too)
c) ParsePixels:
    iptr2 = (unsigned int *) XpmMalloc(sizeof(unsigned int) * width * height);
(width and height are user-supplied)
d) ParseAndPutPixels and ParsePixels:
    cidx[char1][(unsigned char)colorTable[a].string[1]] = a + 1;
(possibly, char1 might be negative, and access the cidx array out of bounds)


Flaw 3. Stack overflow reading pixel values in ParseAndPutPixels (create.c) as
well as ParsePixels (parse.c). Should be exploitable.
This is CAN-2004-0782

A user-supplied number of bytes are stuffed into a fixed-size buffer (typically
8192 bytes). The user gets to choose how many bytes to put into this buffer
via the "number of bytes per pixel" XPM value.
Demo XPM: http://scary.beasts.org/misc/doom2.xpm


CESA-2004-003 - rev 2
Chris Evans
chris@scary.beasts.org