xorl %eax, %eax

ImageMagick XMakeImage() Integer Overflows

leave a comment »

This vulnerability was discovered by Tielei Wang and affects ImageMagick 6.5.2-8 and probably earlier releases. ImageMagick is a widely used graphics library. Here is the vulnerable code as seen in magick/xwindow.c:

13 %                       MagickCore X11 Utility Methods                        %
14 %                                                                             %
15 %                               Software Design                               %
16 %                                 John Cristy                                 %
17 %                                  July 1992                                  %
 ...
5340 %  XMakeImage() creates an X11 image.  If the image size differs from the X11
5341 %  image size, the image is first resized.
 ...
5367 MagickExport MagickBooleanType XMakeImage(Display *display,
5368   const XResourceInfo *resource_info,XWindowInfo *window,Image *image,
5369   unsigned int width,unsigned int height)
5370 {
5371   int
5372     depth,
5373     format;
5374
5375   size_t
5376     length;
5377
5378   XImage
5379     *matte_image,
5380     *ximage;
 ...
5613   if (window->shared_memory == MagickFalse)
5614     {
5615       if (ximage->format == XYBitmap)
5616         length=(size_t) ximage->bytes_per_line*ximage->height*ximage->depth;
5617       else
5618         length=(size_t) ximage->bytes_per_line*ximage->height;
5619       ximage->data=(char *)  malloc(length);
5620     }
 ...
5694             /*
5695               Allocate matte image pixel data.
5696             */
5697             length=(size_t) matte_image->bytes_per_line*
5698               matte_image->height*matte_image->depth;
5699             matte_image->data=(char *) malloc(length);
5700             if (matte_image->data == (char *) NULL)
 ...
5778   return(MagickTrue);
5779 }

The overflows are fairly easy to spot. The length that is calculated at lines 5616, 5618 and 5696 can easily overflow because of the user controlled arguments in the multiplications. To fix those overflows, they added a new macro in this file which is this:

+#define CheckOverflowException(length,width,height) \
+  (((height) != 0) && ((length)/((size_t) height) != ((size_t) width)))
+

And they check the length if there is no X image initialized like this:

+      length=(size_t) ximage->bytes_per_line*ximage->height;
+      if (CheckOverflowException(length,ximage->bytes_per_line,ximage->height))
+        window->shared_memory=MagickFalse;

The previous integer overflows where patched like this:

{
-      if (ximage->format == XYBitmap)
-        length=(size_t) ximage->bytes_per_line*ximage->height*ximage->depth;
+      if (ximage->format != XYBitmap)
+        ximage->data=(char *) AcquireQuantumMemory((size_t)
+          ximage->bytes_per_line,(size_t) ximage->height);
 else
-        length=(size_t) ximage->bytes_per_line*ximage->height;
-      ximage->data=(char *)  malloc(length);
+        ximage->data=(char *) AcquireQuantumMemory((size_t)
+          ximage->bytes_per_line*ximage->depth,(size_t) ximage->height);
 }

And the other one:

 */
-            length=(size_t) matte_image->bytes_per_line*
-              matte_image->height*matte_image->depth;
-            matte_image->data=(char *) malloc(length);
+            matte_image->data=(char *) AcquireQuantumMemory((size_t)
+              matte_image->bytes_per_line*matte_image->depth,
+              (size_t) matte_image->height);
 if (matte_image->data == (char *) NULL)

Written by xorl

May 30, 2009 at 00:15

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s