GRAY: Fix heap-overflow due to tile outside image bounds.
authorBob Friesenhahn <bfriesen@GraphicsMagick.org>
Sun, 03 Dec 2017 11:37:09 -0600
changeset 15284 460ef5e858ad
parent 15283 a9c425688397
child 15285 1366f2dd9931
GRAY: Fix heap-overflow due to tile outside image bounds.
ChangeLog
coders/gray.c
www/Changelog.html
--- a/ChangeLog	Sun Dec 03 09:51:43 2017 -0600
+++ b/ChangeLog	Sun Dec 03 11:37:09 2017 -0600
@@ -1,5 +1,8 @@
 2017-12-03  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
 
+	* coders/gray.c (ReadGRAYImage): Fix SourceForge issue #522
+	"heap-buffer-overflow".  Similar issue to cmyk.c.
+
 	* coders/cmyk.c (ReadCMYKImage): Fix SourceForge issue #521
 	"heap-buffer-overflow". The requested tile must be within the
 	bounds of the image.  As it happens, 'montage' passes size and
--- a/coders/gray.c	Sun Dec 03 09:51:43 2017 -0600
+++ b/coders/gray.c	Sun Dec 03 11:37:09 2017 -0600
@@ -154,14 +154,15 @@
     y;
 
   register long
-    i,
-    x;
+    i;
 
   register PixelPacket
     *q;
 
   size_t
-    count;
+    count,
+    tile_packets,
+    x;
 
   unsigned char
     *scanline;
@@ -194,6 +195,27 @@
   image=AllocateImage(image_info);
   if ((image->columns == 0) || (image->rows == 0))
     ThrowReaderException(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);
+
   status=OpenBlob(image_info,image,ReadBinaryBlobMode,exception);
   if (status == False)
     ThrowReaderException(FileOpenError,UnableToOpenFile,image);
@@ -229,6 +251,8 @@
   scanline=MagickAllocateArray(unsigned char *,packet_size,image->tile_info.width);
   if (scanline == (unsigned char *) NULL)
     ThrowReaderException(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.
   */
@@ -237,19 +261,12 @@
     import_options.endian=image_info->endian;
   if (image->logging)
     (void) LogMagickEvent(CoderEvent,GetMagickModule(),
-                          "Depth: %u bits, "
-                          "Type: %s, "
-                          "Samples/Pixel: %u, "
-                          "Endian %s, "
-                          "Tile: %lux%lu%+ld%+ld",
+                          "Depth: %u bits, Type: %s, "
+                          "Samples/Pixel: %u, Endian %s",
                           quantum_size,
                           QuantumTypeToString(quantum_type),
                           samples_per_pixel,
-                          EndianTypeToString(import_options.endian),
-                          image->tile_info.width,
-                          image->tile_info.height,
-                          image->tile_info.x,
-                          image->tile_info.y);
+                          EndianTypeToString(import_options.endian));
   /*
     Support starting at intermediate image frame.
   */
@@ -261,9 +278,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
   {
     /*
@@ -273,7 +290,8 @@
       if (image->scene >= (image_info->subimage+image_info->subrange-1))
         break;
     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;
     /*
       Support GRAYA with matte channel
     */
@@ -282,7 +300,8 @@
     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=SetImagePixelsEx(image,0,y,image->columns,1,exception);
       if (q == (PixelPacket *) NULL)
         break;
@@ -302,7 +321,8 @@
     image->is_grayscale=is_grayscale;
     count=image->tile_info.height-image->rows-image->tile_info.y;
     for (j=0; j < count; j++)
-      (void) ReadBlob(image,packet_size*image->tile_info.width,scanline);
+      if (ReadBlob(image,tile_packets,scanline) != tile_packets)
+        break;
     if (EOFBlob(image))
       {
         ThrowException(exception,CorruptImageError,UnexpectedEndOfFile,
@@ -315,8 +335,8 @@
     if (image_info->subrange != 0)
       if (image->scene >= (image_info->subimage+image_info->subrange-1))
         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 09:51:43 2017 -0600
+++ b/www/Changelog.html	Sun Dec 03 11:37:09 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/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
 &quot;heap-buffer-overflow&quot;. The requested tile must be within the
 bounds of the image.  As it happens, 'montage' passes size and