Re: GraphicsMagick 1.1.8 release needed

Daniel Kobras <[email protected]> Fri, 18 Aug 2006 12:06:26 +0200
Newsgroups gmane.comp.video.graphicsmagick.core
Message-ID <[email protected]>
On Thu, Aug 17, 2006 at 10:42:35PM -0500, Bob Friesenhahn wrote:
> GraphicsMagick 1.1.7 was released quite some time ago (October, 2005) 
> and there have been many fixes in the CVS version since then.  It 
> appears that we need a 1.1.8 release.
> 
> Please bring my attention to any important fixes which have yet to be 
> applied to the 1.1 branch ASAP.

I'm currently testing a security fix for CVE-2006-4144 (heap overflow in
SGI coder) that should be considered for a new release:

  * coders/sgi.c: Fix multiple heap overflow vulnerabilities in SGI coder
    due to
    + missing boundary checks in SGIDecode();
    + missing validation of pixel depth field;
    + integer overflow via large columns and rows fields (CVE-2006-4144)
      Closes: #383333
    + missing validation of chunk size fields (variable 'runlength') in
      run-length encoded images.
  * coders/sgi.c: Check for bogus values of 'bytes_per_pixel' and 'depth'.
  * coders/sgi.c: Fix calculation of internal depth value.

I'm attaching the patch for inspection, but mind that my test build is still
running.

Regards,

Daniel.

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

_______________________________________________
Graphicsmagick-core mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/graphicsmagick-core
sgi_integer_overflow_fix_CVE-2006-4144 (text/plain, 6 KB)
Fix multiple vulnerabilities in SGI coder (CVE-2006-4144).

--- a/coders/sgi.c	Fri Aug 18 11:25:15 2006 +0200
+++ b/coders/sgi.c	Fri Aug 18 11:48:32 2006 +0200
@@ -144,8 +144,9 @@ static unsigned int IsSGI(const unsigned
 %
 */
 
-static void SGIDecode(const unsigned long bytes_per_pixel,
-  unsigned char *max_packets,unsigned char *pixels)
+static int SGIDecode(const unsigned long bytes_per_pixel,
+  unsigned char *max_packets,unsigned char *pixels,
+  unsigned long npackets,unsigned long npixels)
 {
   long
     count;
@@ -163,20 +164,29 @@ static void SGIDecode(const unsigned lon
     {
       for ( ; ; )
       {
+        if (npackets-- == 0)
+	  return -1;
         pixel=(*p++) << 8;
         pixel|=(*p++);
         count=(long) (pixel & 0x7f);
         if (count == 0)
           break;
+	if (count > npixels)
+	  return -1;
+	npixels -= count;
         if (pixel & 0x80)
           for ( ; count != 0; count--)
           {
+	    if (npackets-- == 0)
+	      return -1;
             *q=(*p++);
             *(q+1)=(*p++);
             q+=8;
           }
         else
           {
+	    if (npackets-- == 0)
+	      return -1;
             pixel=(*p++) << 8;
             pixel|=(*p++);
             for ( ; count != 0; count--)
@@ -187,22 +197,31 @@ static void SGIDecode(const unsigned lon
             }
           }
       }
-      return;
+      return 0;
     }
   for ( ; ; )
   {
+    if (npackets-- == 0)
+      return -1;
     pixel=(*p++);
     count=(long) (pixel & 0x7f);
     if (count == 0)
       break;
+    if (count > npixels)
+      return -1;
+    npixels -= count;
     if (pixel & 0x80)
       for ( ; count != 0; count--)
       {
+        if (npackets-- == 0)
+          return -1;
         *q=(*p++);
         q+=4;
       }
     else
       {
+        if (npackets-- == 0)
+          return -1;
         pixel=(*p++);
         for ( ; count != 0; count--)
         {
@@ -211,6 +230,7 @@ static void SGIDecode(const unsigned lon
         }
       }
   }
+  return 0;
 }
 
 static Image *ReadSGIImage(const ImageInfo *image_info,ExceptionInfo *exception)
@@ -274,18 +294,22 @@ static Image *ReadSGIImage(const ImageIn
     if (iris_info.storage == 0x01)
       image->compression=RLECompression;
     iris_info.bytes_per_pixel=ReadBlobByte(image);
+    if (iris_info.bytes_per_pixel > 2 || iris_info.bytes_per_pixel == 0)
+      ThrowReaderException(CorruptImageError,ImproperImageHeader,image);
     iris_info.dimension=ReadBlobMSBShort(image);
     iris_info.columns=ReadBlobMSBShort(image);
     iris_info.rows=ReadBlobMSBShort(image);
     iris_info.depth=ReadBlobMSBShort(image);
+    if (iris_info.depth > 4 || iris_info.depth == 0)
+      ThrowReaderException(CorruptImageError,ImproperImageHeader,image);
+    if (iris_info.depth < 3)
+      {
+        image->storage_class=PseudoClass;
+        image->colors=256;
+      }
     image->columns=iris_info.columns;
     image->rows=iris_info.rows;
-    image->depth=iris_info.depth <= 8 ? 8 : QuantumDepth;
-    if (iris_info.depth < 3)
-      {
-        image->storage_class=PseudoClass;
-        image->colors=256;
-      }
+    image->depth=bytes_per_pixel == 1 ? 8 : QuantumDepth;
     if (image_info->ping && (image_info->subrange != 0))
       if (image->scene >= (image_info->subimage+image_info->subrange-1))
         break;
@@ -298,6 +322,8 @@ static Image *ReadSGIImage(const ImageIn
     */
     bytes_per_pixel=iris_info.bytes_per_pixel;
     number_pixels=iris_info.columns*iris_info.rows;
+    if (4*bytes_per_pixel*number_pixels < number_pixels) /* Overflow? */
+      ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,image);
     iris_pixels=MagickAllocateMemory(unsigned char *,
       4*bytes_per_pixel*number_pixels);
     if (iris_pixels == (unsigned char *) NULL)
@@ -368,7 +394,11 @@ static Image *ReadSGIImage(const ImageIn
         for (i=0; i < (int) (iris_info.rows*iris_info.depth); i++)
           offsets[i]=ReadBlobMSBLong(image);
         for (i=0; i < (int) (iris_info.rows*iris_info.depth); i++)
-          runlength[i]=ReadBlobMSBLong(image);
+	  {
+            runlength[i]=ReadBlobMSBLong(image);
+	    if (runlength[i] > 4*iris_info.columns+10)
+	      ThrowReaderException(CorruptImageError,ImproperImageHeader,image); 
+	  }
         /*
           Check data order.
         */
@@ -381,7 +411,7 @@ static Image *ReadSGIImage(const ImageIn
               data_order=1;
             offset=offsets[y+z*iris_info.rows];
           }
-        offset=512+4*bytes_per_pixel*2*(iris_info.rows*iris_info.depth);
+        offset=TellBlob(image);
         if (data_order == 1)
           {
             for (z=0; z < (int) iris_info.depth; z++)
@@ -397,7 +427,11 @@ static Image *ReadSGIImage(const ImageIn
                 (void) ReadBlob(image,runlength[y+z*iris_info.rows],
                   (char *) max_packets);
                 offset+=runlength[y+z*iris_info.rows];
-                SGIDecode(bytes_per_pixel,max_packets,p+bytes_per_pixel*z);
+                if (SGIDecode(bytes_per_pixel,max_packets,p+bytes_per_pixel*z,
+		              runlength[y+z*iris_info.rows]/bytes_per_pixel,
+                              iris_info.columns) == -1)
+	          ThrowReaderException(CorruptImageError,ImproperImageHeader,
+		                       image); 
                 p+=(iris_info.columns*4*bytes_per_pixel);
               }
             }
@@ -417,7 +451,11 @@ static Image *ReadSGIImage(const ImageIn
                 (void) ReadBlob(image,runlength[y+z*iris_info.rows],
                   (char *) max_packets);
                 offset+=runlength[y+z*iris_info.rows];
-                SGIDecode(bytes_per_pixel,max_packets,p+bytes_per_pixel*z);
+                if (SGIDecode(bytes_per_pixel,max_packets,p+bytes_per_pixel*z,
+		              runlength[y+z*iris_info.rows]/bytes_per_pixel,
+                              iris_info.columns) == -1)
+	          ThrowReaderException(CorruptImageError,ImproperImageHeader,
+		                       image); 
               }
               p+=(iris_info.columns*4*bytes_per_pixel);
             }