RGB: Fix heap-overflow due to tile outside image bounds.
authorBob Friesenhahn <bfriesen@GraphicsMagick.org>
Sun, 03 Dec 2017 12:51:20 -0600
changeset 15285 1366f2dd9931
parent 15284 460ef5e858ad
child 15286 6c9e1aee917b
RGB: Fix heap-overflow due to tile outside image bounds.
ChangeLog
coders/rgb.c
www/Changelog.html
--- a/ChangeLog	Sun Dec 03 11:37:09 2017 -0600
+++ b/ChangeLog	Sun Dec 03 12:51:20 2017 -0600
@@ -1,5 +1,8 @@
 2017-12-03  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
 
+	* coders/rgb.c (ReadRGBImage): Fix SourceForge issue #523
+	"heap-buffer-overflow".  Similar issue to cmyk.c.
+
 	* coders/gray.c (ReadGRAYImage): Fix SourceForge issue #522
 	"heap-buffer-overflow".  Similar issue to cmyk.c.
 
--- a/coders/rgb.c	Sun Dec 03 11:37:09 2017 -0600
+++ b/coders/rgb.c	Sun Dec 03 12:51:20 2017 -0600
@@ -96,14 +96,15 @@
     y;
 
   register long
-    i,
-    x;
+    i;
 
   register PixelPacket
     *q;
 
   size_t
-    count;
+    count,
+    tile_packets,
+    x;
 
   unsigned char
     *scanline = (unsigned char *) NULL;
@@ -125,6 +126,27 @@
   image=AllocateImage(image_info);
   if ((image->columns == 0) || (image->rows == 0))
     ThrowRGBReaderException(OptionError,MustSpecifyImageSize,image);
+  if (image->logging)
+    (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                          "Tile %lux%lu%+ld%+ld, Offset %lu",
+                          image->tile_info.width,image->tile_info.height,
+                          image->tile_info.x,image->tile_info.y,
+                          image->offset);
+  /*
+    There is the option to either require that the tile be within the
+    image bounds or to return only the portion of the tile which is
+    within the image bounds (returned image is smaller than requested
+    tile size).  For the moment we choose the former.
+  */
+  if ((image->tile_info.width > image->columns) ||
+      (image->tile_info.x < 0) ||
+      (image->tile_info.width+image->tile_info.x > image->columns) ||
+      (image->tile_info.height > image->rows) ||
+      (image->tile_info.y < 0) ||
+      (image->tile_info.height+image->tile_info.y > image->rows)
+      )
+    ThrowReaderException(OptionError,TileNotBoundedByImageDimensions,image);
+
   if (image_info->interlace != PartitionInterlace)
     {
       /*
@@ -168,6 +190,8 @@
                                packet_size,image->tile_info.width);
   if (scanline == (unsigned char *) NULL)
     ThrowRGBReaderException(ResourceLimitError,MemoryAllocationFailed,image);
+  tile_packets=(size_t) packet_size*image->tile_info.width;
+  x=(size_t) (packet_size*image->tile_info.x);
   /*
     Initialize import options.
   */
@@ -192,9 +216,9 @@
       */
       image->scene++;
       for (y=0; y < (long) image->rows; y++)
-        (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+        if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+          break;
     }
-  x=(long) (packet_size*image->tile_info.x);
   do
   {
     /*
@@ -216,11 +240,13 @@
         */
         quantum_type=(image->matte ? RGBAQuantum : RGBQuantum);
         for (y=0; y < image->tile_info.y; y++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         for (y=0; y < (long) image->rows; y++)
         {
           if ((y > 0) || (image->previous == (Image *) NULL))
-            (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+            if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+              break;
           q=SetImagePixels(image,0,y,image->columns,1);
           if (q == (PixelPacket *) NULL)
             break;
@@ -237,7 +263,8 @@
         }
         count=image->tile_info.height-image->rows-image->tile_info.y;
         for (i=0; i < (long) count; i++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         break;
       }
       case LineInterlace:
@@ -247,26 +274,30 @@
         */
         packet_size=(quantum_size)/8;
         for (y=0; y < image->tile_info.y; y++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         for (y=0; y < (long) image->rows; y++)
         {
           if ((y > 0) || (image->previous == (Image *) NULL))
-            (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+            if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+              break;
           q=SetImagePixels(image,0,y,image->columns,1);
           if (q == (PixelPacket *) NULL)
             break;
           (void) ImportImagePixelArea(image,RedQuantum,quantum_size,scanline+x,
                                       &import_options,0);
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
           (void) ImportImagePixelArea(image,GreenQuantum,quantum_size,scanline+x,
                                       &import_options,0);
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
           (void) ImportImagePixelArea(image,BlueQuantum,quantum_size,scanline+x,
                                       &import_options,0);
           if (image->matte)
             {
-              (void) ReadBlob(image,packet_size*image->tile_info.width,
-                scanline);
+              if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+                break;
               (void) ImportImagePixelArea(image,AlphaQuantum,quantum_size,scanline+x,
                                           &import_options,0);
             }
@@ -281,7 +312,8 @@
         }
         count=image->tile_info.height-image->rows-image->tile_info.y;
         for (i=0; i < (long) count; i++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         break;
       }
       case PlaneInterlace:
@@ -302,13 +334,15 @@
           }
         packet_size=(quantum_size)/8;
         for (y=0; y < image->tile_info.y; y++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         i=0;
         span=image->rows*(image->matte ? 4 : 3);
         for (y=0; y < (long) image->rows; y++)
         {
           if ((y > 0) || (image->previous == (Image *) NULL))
-            (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+            if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+              break;
           q=SetImagePixels(image,0,y,image->columns,1);
           if (q == (PixelPacket *) NULL)
             break;
@@ -326,7 +360,8 @@
         }
         count=image->tile_info.height-image->rows-image->tile_info.y;
         for (i=0; i < (long) count; i++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         if (image_info->interlace == PartitionInterlace)
           {
             CloseBlob(image);
@@ -336,10 +371,12 @@
               ThrowRGBReaderException(FileOpenError,UnableToOpenFile,image);
           }
         for (y=0; y < image->tile_info.y; y++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         for (y=0; y < (long) image->rows; y++)
         {
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
           q=GetImagePixels(image,0,y,image->columns,1);
           if (q == (PixelPacket *) NULL)
             break;
@@ -357,7 +394,8 @@
         }
         count=image->tile_info.height-image->rows-image->tile_info.y;
         for (i=0; i < (long) count; i++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         if (image_info->interlace == PartitionInterlace)
           {
             CloseBlob(image);
@@ -367,10 +405,12 @@
               ThrowRGBReaderException(FileOpenError,UnableToOpenFile,image);
           }
         for (y=0; y < image->tile_info.y; y++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         for (y=0; y < (long) image->rows; y++)
         {
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
           q=GetImagePixels(image,0,y,image->columns,1);
           if (q == (PixelPacket *) NULL)
             break;
@@ -388,7 +428,8 @@
         }
         count=image->tile_info.height-image->rows-image->tile_info.y;
         for (i=0; i < (long) count; i++)
-          (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+          if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+            break;
         if (image->matte)
           {
             /*
@@ -403,12 +444,12 @@
                   ThrowRGBReaderException(FileOpenError,UnableToOpenFile,image);
               }
             for (y=0; y < image->tile_info.y; y++)
-              (void) ReadBlob(image,packet_size*image->tile_info.width,
-                scanline);
+              if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+                break;
             for (y=0; y < (long) image->rows; y++)
             {
-              (void) ReadBlob(image,packet_size*image->tile_info.width,
-                scanline);
+              if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+                break;
               q=GetImagePixels(image,0,y,image->columns,1);
               if (q == (PixelPacket *) NULL)
                 break;
@@ -426,8 +467,8 @@
             }
             count=image->tile_info.height-image->rows-image->tile_info.y;
             for (i=0; i < (long) count; i++)
-              (void) ReadBlob(image,packet_size*image->tile_info.width,
-                scanline);
+              if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+                break;
           }
         if (image_info->interlace == PartitionInterlace)
           (void) strlcpy(image->filename,image_info->filename,MaxTextExtent);
@@ -448,8 +489,8 @@
         break;
     if (image_info->interlace == PartitionInterlace)
       break;
-    count=ReadBlob(image,packet_size*image->tile_info.width,scanline);
-    if (count != 0)
+    count=ReadBlob(image,tile_packets,scanline);
+    if (count == tile_packets)
       {
         /*
           Allocate next image structure.
--- a/www/Changelog.html	Sun Dec 03 11:37:09 2017 -0600
+++ b/www/Changelog.html	Sun Dec 03 12:51:20 2017 -0600
@@ -38,6 +38,8 @@
 <p>2017-12-03  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">
+<li>coders/rgb.c (ReadRGBImage): Fix SourceForge issue #523
+&quot;heap-buffer-overflow&quot;.  Similar issue to cmyk.c.</li>
 <li>coders/gray.c (ReadGRAYImage): Fix SourceForge issue #522
 &quot;heap-buffer-overflow&quot;.  Similar issue to cmyk.c.</li>
 <li>coders/cmyk.c (ReadCMYKImage): Fix SourceForge issue #521