tag 413040 + patch
clone 413040 -1
clone 413040 -2
reassign -1 libx11 2:1.0.3-5
retitle -1 libX11: Buffer overflow in XGetPixel().
severity -1 critical
tag -1 + patch
tag -1 + security
reassign -2 xlibs 4.3.0.dfsg.1-14sarge3
retitle -2 libX11: Buffer overflow in XGetPixel().
severity -2 critical
tag -2 + patch
tag -2 + security
thanks
On Thu, Mar 01, 2007 at 09:01:48PM +0100, Daniel Kobras wrote:
On Thu, Mar 01, 2007 at 05:37:39AM +0200, Sami Liedes wrote:
The attached files all crash imagemagick (eg. XXXtojpg $filename) on
amd64, some with SEGV, some with glibc detected heap corruption. I
consider it quite likely that some of these are exploitable, but as
I'm not sure, only filing as Severity: normal as to not annoy you :)
Thanks. I've done a quick screening to investigate which of those affect
graphicsmagick, and have cloned individual bugs as I'm probably unable
to deal with all of them in one go. Bug severity might change once I've
had a closer look at the individual issues. Here's the detailed list for
current graphicsmagick:
(...)
Broken conversion
=
The following coders show no problems on gm identify, but break with
gm convert to jpg and gif.
(...)
xwd:
broken.xwd ... Segmentation fault
While not a problem in imagemagick/graphicsmagick themselves, this turns
out as the most grave bug the testcases have uncovered so far. The
*magick XWD coder initializes an XImage structure from a user-supplied
image in X window dump format, passes it to libX11's XInitImage() for
validation, properly checking its return value. Later on it uses
XGetPixel() to obtain individual pixel values.
The broken.xwd testcase supplied in the original bug report contains
specifies a very large bit_per_pixel value. XInitImage() does not check
this case and validates the image. Calling XGetPixel() with the bogus
XImage structure causes an overflow of the px variable that is allocated
on the stack of function _XGetPixel() in src/ImUtil.c. Similar scenarios
arise for different image variants if the bitmap_unit member of XImage
contains excessively large values. The first attached patch is
completely untested so please check before applying. It adds more sanity
checks to XInitImage() to prevent the described buffer overflows in
XGetPixel(). I haven't considered other code paths, so the patch might
not be comprehensive. The second attached patch extends the
graphicsmagick code by a few more sanity checks to plug the hole even
with the present libX11, but from my reading of XInitImage man page,
libX11 ought to take of this itself.
I've done the analysis on an etch system, identical code is already
present in the affected functions in xfree86 on sarge, though. Looks
like stable requires an update as well.
Regards,
Daniel.
--- src/ImUtil.c.orig 2007-03-08 21:24:13.0 +0100
+++ src/ImUtil.c 2007-03-08 21:28:22.0 +0100
@@ -385,6 +385,8 @@
XImage *image;
{
if (image-depth == 0 || image-depth 32 ||
+ image-bits_per_pixel 32 || image-bitmap_unit 32 ||
+ image-bits_per_pixel 0 || image-bitmap_unit 0 ||
(image-format != XYBitmap
image-format != XYPixmap
image-format != ZPixmap) ||
--- a/coders/xwd.c Tue Mar 06 08:34:38 2007 +0100
+++ b/coders/xwd.c Thu Mar 08 21:13:04 2007 +0100
@@ -239,6 +239,13 @@ static Image *ReadXWDImage(const ImageIn
ximage-red_mask=header.red_mask;
ximage-green_mask=header.green_mask;
ximage-blue_mask=header.blue_mask;
+ /* Why those are signed ints is beyond me. */
+ if (ximage-depth 0 || ximage-width 0 || ximage-height 0 ||
+ ximage-bitmap_pad 0 || ximage-bytes_per_line 0)
+ThrowReaderException(CorruptImageError,ImproperImageHeader,image);
+ /* Guard against buffer overflow in libX11. */
+ if (ximage-bits_per_pixel 32 || ximage-bitmap_unit 32)
+ThrowReaderException(CorruptImageError,ImproperImageHeader,image);
status=XInitImage(ximage);
if (status == False)
ThrowReaderException(CorruptImageError,UnrecognizedXWDHeader,image);
