CVE-2017-9144: Improper Input Validation (CWE-20)
value: In ImageMagick 7.0.5-5, a crafted RLE image can trigger a crash because of incorrect EOF handling in coders/rle.c.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 6.5
Published: 2017-05-22T14:29Z
Repository: https://github.com/ImageMagick/ImageMagick
Language: Other
Files changed: ChangeLog, coders/rle.c
Commit date: 2017-05-12 07:13:08 -0400
Commit message: Check for EOF conditions for RLE image format

Diff: diff --git a/ChangeLog b/ChangeLog
index 8435448406e..3d934d28e2f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,10 +9,11 @@
     https://github.com/ImageMagick/ImageMagick/issues/463).
   * The +opaque option now works on all channels (reference
     https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31862).
   * Ensure backwards compatibility for the -combine option (reference
     https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31855).
+  * Check for EOF conditions for RLE image format.
 
 2017-04-24  7.0.5-5 Cristy  <quetzlzacatenango@image...>
   * Release ImageMagick version 7.0.5-5, GIT revision 19915:12eec43:20170424.
 
 2017-03-26  7.0.5-5 Cristy  <quetzlzacatenango@image...>
diff --git a/coders/rle.c b/coders/rle.c
index cfb20f556f0..e2d68ead36b 100644
--- a/coders/rle.c
+++ b/coders/rle.c
@@ -130,10 +130,18 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
 #define SetColorOp  0x02
 #define SkipPixelsOp  0x03
 #define ByteDataOp  0x05
 #define RunDataOp  0x06
 #define EOFOp  0x07
+#define ThrowRLEException(exception,message) \
+{ \
+  if (colormap != (unsigned char *) NULL) \
+    colormap=(unsigned char *) RelinquishMagickMemory(colormap); \
+  if (pixel_info != (MemoryInfo *) NULL) \
+    pixel_info=RelinquishVirtualMemory(pixel_info); \
+  ThrowReaderException((exception),(message)); \
+}
 
   char
     magick[12];
 
   Image
@@ -204,30 +212,34 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
   if (status == MagickFalse)
     return(DestroyImageList(image));
   /*
     Determine if this a RLE file.
   */
+  colormap=(unsigned char *) NULL;
+  pixel_info=(MemoryInfo *) NULL;
   count=ReadBlob(image,2,(unsigned char *) magick);
   if ((count != 2) || (memcmp(magick,"\122\314",2) != 0))
     ThrowReaderException(CorruptImageError,"ImproperImageHeader");
   do
   {
     /*
       Read image header.
     */
-    image->page.x=ReadBlobLSBShort(image);
-    image->page.y=ReadBlobLSBShort(image);
+    image->page.x=(ssize_t) ReadBlobLSBShort(image);
+    image->page.y=(ssize_t) ReadBlobLSBShort(image);
     image->columns=ReadBlobLSBShort(image);
     image->rows=ReadBlobLSBShort(image);
     flags=(MagickStatusType) ReadBlobByte(image);
     image->alpha_trait=flags & 0x04 ? BlendPixelTrait : UndefinedPixelTrait;
     number_planes=(size_t) ReadBlobByte(image);
     bits_per_pixel=(size_t) ReadBlobByte(image);
     number_colormaps=(size_t) ReadBlobByte(image);
     map_length=(unsigned char) ReadBlobByte(image);
     if (map_length >= 22)
       ThrowReaderException(CorruptImageError,"ImproperImageHeader");
+    if (EOFBlob(image) != MagickFalse)
+      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
     one=1;
     map_length=one << map_length;
     if ((number_planes == 0) || (number_planes == 2) ||
         ((flags & 0x04) && (number_colormaps > 254)) || (bits_per_pixel != 8) ||
         (image->columns == 0))
@@ -251,16 +263,11 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
           *p++=(unsigned char) ReadBlobByte(image);
       }
     if ((number_planes & 0x01) == 0)
       (void) ReadBlobByte(image);
     if (EOFBlob(image) != MagickFalse)
-      {
-        ThrowFileException(exception,CorruptImageError,"UnexpectedEndOfFile",
-          image->filename);
-        break;
-      }
-    colormap=(unsigned char *) NULL;
+      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
     if (number_colormaps != 0)
       {
         /*
           Read image colormaps.
         */
@@ -269,12 +276,16 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
         if (colormap == (unsigned char *) NULL)
           ThrowReaderException(ResourceLimitError,"MemoryAllocationFailed");
         p=colormap;
         for (i=0; i < (ssize_t) number_colormaps; i++)
           for (x=0; x < (ssize_t) map_length; x++)
+	  {
             *p++=(unsigned char) ScaleQuantumToChar(ScaleShortToQuantum(
               ReadBlobLSBShort(image)));
+            if (EOFBlob(image) != MagickFalse)
+              ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
+	  }
       }
     if ((flags & 0x08) != 0)
       {
         char
           *comment;
@@ -298,15 +309,11 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
             if ((length & 0x01) == 0)
               (void) ReadBlobByte(image);
           }
       }
     if (EOFBlob(image) != MagickFalse)
-      {
-        ThrowFileException(exception,CorruptImageError,"UnexpectedEndOfFile",
-          image->filename);
-        break;
-      }
+      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
     if ((image_info->ping != MagickFalse) && (image_info->number_scenes != 0))
       if (image->scene >= (image_info->scene+image_info->number_scenes-1))
         break;
     status=SetImageExtent(image,image->columns,image->rows,exception);
     if (status == MagickFalse)
@@ -357,50 +364,72 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
     */
     plane=0;
     x=0;
     y=0;
     opcode=ReadBlobByte(image);
+    if (opcode == EOF)
+      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
     do
     {
       switch (opcode & 0x3f)
       {
         case SkipLinesOp:
         {
           operand=ReadBlobByte(image);
+          if (opcode == EOF)
+            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
           if (opcode & 0x40)
-            operand=ReadBlobLSBSignedShort(image);
+            {
+              operand=ReadBlobLSBSignedShort(image);
+              if (opcode == EOF)
+                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
+	    }
           x=0;
           y+=operand;
           break;
         }
         case SetColorOp:
         {
           operand=ReadBlobByte(image);
+          if (opcode == EOF)
+            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
           plane=(unsigned char) operand;
           if (plane == 255)
             plane=(unsigned char) (number_planes-1);
           x=0;
           break;
         }
         case SkipPixelsOp:
         {
           operand=ReadBlobByte(image);
+          if (opcode == EOF)
+            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
           if (opcode & 0x40)
-            operand=ReadBlobLSBSignedShort(image);
+            {
+              operand=ReadBlobLSBSignedShort(image);
+              if (opcode == EOF)
+                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
+            }
           x+=operand;
           break;
         }
         case ByteDataOp:
         {
           operand=ReadBlobByte(image);
+          if (opcode == EOF)
+            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
           if (opcode & 0x40)
-            operand=ReadBlobLSBSignedShort(image);
-          offset=((image->rows-y-1)*image->columns*number_planes)+x*
-            number_planes+plane;
+            {
+              operand=ReadBlobLSBSignedShort(image);
+              if (opcode == EOF)
+                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
+            }
+          offset=(ssize_t) (((image->rows-y-1)*image->columns*number_planes)+x*
+            number_planes+plane);
           operand++;
           if ((offset < 0) ||
-              (offset+((size_t) operand*number_planes) > pixel_info_length))
+              ((offset+operand*number_planes) > (ssize_t) pixel_info_length))
             {
               if (number_colormaps != 0)
                 colormap=(unsigned char *) RelinquishMagickMemory(colormap);
               pixel_info=RelinquishVirtualMemory(pixel_info);
               ThrowReaderException(CorruptImageError,"UnableToReadImageData");
@@ -420,19 +449,25 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
           break;
         }
         case RunDataOp:
         {
           operand=ReadBlobByte(image);
+          if (opcode == EOF)
+            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
           if (opcode & 0x40)
-            operand=ReadBlobLSBSignedShort(image);
+            {
+              operand=ReadBlobLSBSignedShort(image);
+              if (opcode == EOF)
+                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
+            }
           pixel=(unsigned char) ReadBlobByte(image);
           (void) ReadBlobByte(image);
-          offset=((image->rows-y-1)*image->columns*number_planes)+x*
-            number_planes+plane;
+          offset=(ssize_t) (((image->rows-y-1)*image->columns*number_planes)+x*
+            number_planes+plane);
           operand++;
           if ((offset < 0) ||
-              (offset+((size_t) operand*number_planes) > pixel_info_length))
+              ((offset+operand*number_planes) > (ssize_t) pixel_info_length))
             {
               if (number_colormaps != 0)
                 colormap=(unsigned char *) RelinquishMagickMemory(colormap);
               pixel_info=RelinquishVirtualMemory(pixel_info);
               ThrowReaderException(CorruptImageError,"UnableToReadImageData");
@@ -450,10 +485,12 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
         }
         default:
           break;
       }
       opcode=ReadBlobByte(image);
+      if (opcode == EOF)
+        ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
     } while (((opcode & 0x3f) != EOFOp) && (opcode != EOF));
     if (number_colormaps != 0)
       {
         MagickStatusType
           mask;
@@ -465,20 +502,20 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
         p=pixels;
         x=(ssize_t) number_planes;
         if (number_colormaps == 1)
           for (i=0; i < (ssize_t) number_pixels; i++)
           {
-            ValidateColormapValue(image,*p & mask,&index,exception);
+            ValidateColormapValue(image,(ssize_t) (*p & mask),&index,exception);
             *p=colormap[(ssize_t) index];
             p++;
           }
         else
           if ((number_planes >= 3) && (number_colormaps >= 3))
             for (i=0; i < (ssize_t) number_pixels; i++)
               for (x=0; x < (ssize_t) number_planes; x++)
               {
-                ValidateColormapValue(image,(size_t) (x*map_length+
+                ValidateColormapValue(image,(ssize_t) (x*map_length+
                   (*p & mask)),&index,exception);
                 *p=colormap[(ssize_t) index];
                 p++;
               }
         if ((i < (ssize_t) number_pixels) || (x < (ssize_t) number_planes))
@@ -568,11 +605,11 @@ static Image *ReadRLEImage(const ImageInfo *image_info,ExceptionInfo *exception)
               q=QueueAuthenticPixels(image,0,y,image->columns,1,exception);
               if (q == (Quantum *) NULL)
                 break;
               for (x=0; x < (ssize_t) image->columns; x++)
               {
-                SetPixelIndex(image,*p++,q);
+                SetPixelIndex(image,(Quantum) *p++,q);
                 q+=GetPixelChannels(image);
               }
               if (SyncAuthenticPixels(image,exception) == MagickFalse)
                 break;
               if (image->previous == (Image *) NULL)

Vulnerable code:     image->page.x=ReadBlobLSBShort(image);
    image->page.y=ReadBlobLSBShort(image);
      {
        ThrowFileException(exception,CorruptImageError,"UnexpectedEndOfFile",
          image->filename);
        break;
      }
    colormap=(unsigned char *) NULL;
      {
        ThrowFileException(exception,CorruptImageError,"UnexpectedEndOfFile",
          image->filename);
        break;
      }
            operand=ReadBlobLSBSignedShort(image);
            operand=ReadBlobLSBSignedShort(image);
            operand=ReadBlobLSBSignedShort(image);
          offset=((image->rows-y-1)*image->columns*number_planes)+x*
            number_planes+plane;
              (offset+((size_t) operand*number_planes) > pixel_info_length))
            operand=ReadBlobLSBSignedShort(image);
          offset=((image->rows-y-1)*image->columns*number_planes)+x*
            number_planes+plane;
              (offset+((size_t) operand*number_planes) > pixel_info_length))
            ValidateColormapValue(image,*p & mask,&index,exception);
                ValidateColormapValue(image,(size_t) (x*map_length+
                SetPixelIndex(image,*p++,q);
Fixed code:   * Check for EOF conditions for RLE image format.
#define ThrowRLEException(exception,message) \
{ \
  if (colormap != (unsigned char *) NULL) \
    colormap=(unsigned char *) RelinquishMagickMemory(colormap); \
  if (pixel_info != (MemoryInfo *) NULL) \
    pixel_info=RelinquishVirtualMemory(pixel_info); \
  ThrowReaderException((exception),(message)); \
}
  colormap=(unsigned char *) NULL;
  pixel_info=(MemoryInfo *) NULL;
    image->page.x=(ssize_t) ReadBlobLSBShort(image);
    image->page.y=(ssize_t) ReadBlobLSBShort(image);
    if (EOFBlob(image) != MagickFalse)
      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
	  {
            if (EOFBlob(image) != MagickFalse)
              ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
	  }
      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
    if (opcode == EOF)
      ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
          if (opcode == EOF)
            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            {
              operand=ReadBlobLSBSignedShort(image);
              if (opcode == EOF)
                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
	    }
          if (opcode == EOF)
            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
          if (opcode == EOF)
            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            {
              operand=ReadBlobLSBSignedShort(image);
              if (opcode == EOF)
                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            }
          if (opcode == EOF)
            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            {
              operand=ReadBlobLSBSignedShort(image);
              if (opcode == EOF)
                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            }
          offset=(ssize_t) (((image->rows-y-1)*image->columns*number_planes)+x*
            number_planes+plane);
              ((offset+operand*number_planes) > (ssize_t) pixel_info_length))
          if (opcode == EOF)
            ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            {
              operand=ReadBlobLSBSignedShort(image);
              if (opcode == EOF)
                ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            }
          offset=(ssize_t) (((image->rows-y-1)*image->columns*number_planes)+x*
            number_planes+plane);
              ((offset+operand*number_planes) > (ssize_t) pixel_info_length))
      if (opcode == EOF)
        ThrowRLEException(CorruptImageError,"UnexpectedEndOfFile");
            ValidateColormapValue(image,(ssize_t) (*p & mask),&index,exception);
                ValidateColormapValue(image,(ssize_t) (x*map_length+
                SetPixelIndex(image,(Quantum) *p++,q);