CVE-2016-5688: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The WPG parser in ImageMagick before 6.9.4-4 and 7.x before 7.0.1-5, when a memory limit is set, allows remote attackers to have unspecified impact via vectors related to the SetImageExtent return-value check, which trigger (1) a heap-based buffer overflow in the SetPixelIndex function or an invalid write operation in the (2) ScaleCharToQuantum or (3) SetPixelIndex functions.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 8.1
Published: 2016-12-13T15:59Z
Repository: https://github.com/ImageMagick/ImageMagick
Language: C
Files changed: MagickCore/cache.c, MagickCore/image.c, coders/wpg.c
Commit date: 2016-05-16 16:05:02 -0400
Commit message: Set pixel cache to undefined if any resource limit is exceeded

Diff: diff --git a/MagickCore/cache.c b/MagickCore/cache.c
index 2f671608f6..921ddbf27e 100644
--- a/MagickCore/cache.c
+++ b/MagickCore/cache.c
@@ -3502,10 +3502,11 @@ static MagickBooleanType OpenPixelCache(Image *image,const MapMode mode,
                 }
               return(MagickTrue);
             }
         }
       RelinquishMagickResource(DiskResource,cache_info->length);
+      cache_info->type=UndefinedCache;
       (void) ThrowMagickException(exception,GetMagickModule(),CacheError,
         "CacheResourcesExhausted","`%s'",image->filename);
       return(MagickFalse);
     }
   if ((source_info.storage_class != UndefinedClass) && (mode != ReadMode))
@@ -3514,18 +3515,20 @@ static MagickBooleanType OpenPixelCache(Image *image,const MapMode mode,
       *cache_info->cache_filename='\0';
     }
   if (OpenPixelCacheOnDisk(cache_info,mode) == MagickFalse)
     {
       RelinquishMagickResource(DiskResource,cache_info->length);
+      cache_info->type=UndefinedCache;
       ThrowFileException(exception,CacheError,"UnableToOpenPixelCache",
         image->filename);
       return(MagickFalse);
     }
   status=SetPixelCacheExtent(image,(MagickSizeType) cache_info->offset+
     cache_info->length);
   if (status == MagickFalse)
     {
+      cache_info->type=UndefinedCache;
       ThrowFileException(exception,CacheError,"UnableToExtendCache",
         image->filename);
       return(MagickFalse);
     }
   length=number_pixels*(cache_info->number_channels*sizeof(Quantum)+
@@ -3580,10 +3583,12 @@ static MagickBooleanType OpenPixelCache(Image *image,const MapMode mode,
                     cache_info->rows,(double) cache_info->number_channels,
                     format);
                   (void) LogMagickEvent(CacheEvent,GetMagickModule(),"%s",
                     message);
                 }
+              if (status == MagickFalse)
+                cache_info->type=UndefinedCache;
               return(status);
             }
         }
       RelinquishMagickResource(MapResource,cache_info->length);
     }
@@ -3604,10 +3609,12 @@ static MagickBooleanType OpenPixelCache(Image *image,const MapMode mode,
         cache_info->cache_filename,cache_info->file,type,(double)
         cache_info->columns,(double) cache_info->rows,(double)
         cache_info->number_channels,format);
       (void) LogMagickEvent(CacheEvent,GetMagickModule(),"%s",message);
     }
+  if (status == MagickFalse)
+    cache_info->type=UndefinedCache;
   return(status);
 }
 
 /*
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
diff --git a/MagickCore/image.c b/MagickCore/image.c
index 1b35195713..a41ad0cc60 100644
--- a/MagickCore/image.c
+++ b/MagickCore/image.c
@@ -841,11 +841,14 @@ MagickExport Image *CloneImage(const Image *image,const size_t columns,
       clone_image->colors=image->colors;
       length=(size_t) image->colors;
       clone_image->colormap=(PixelInfo *) AcquireQuantumMemory(length,
         sizeof(*clone_image->colormap));
       if (clone_image->colormap == (PixelInfo *) NULL)
-        ThrowImageException(ResourceLimitError,"MemoryAllocationFailed");
+        {
+          clone_image=DestroyImage(clone_image);
+          ThrowImageException(ResourceLimitError,"MemoryAllocationFailed");
+        }
       (void) CopyMagickMemory(clone_image->colormap,image->colormap,length*
         sizeof(*clone_image->colormap));
     }
   clone_image->image_info=CloneImageInfo(image->image_info);
   (void) CloneImageProfiles(clone_image,image);
diff --git a/coders/wpg.c b/coders/wpg.c
index 0dfdc47f0b..782f25e827 100644
--- a/coders/wpg.c
+++ b/coders/wpg.c
@@ -1309,11 +1309,10 @@ static Image *ReadWPGImage(const ImageInfo *image_info,
               image->columns=Bitmap2Header1.Width;
               image->rows=Bitmap2Header1.Height;
               status=SetImageExtent(image,image->columns,image->rows,exception);
               if (status == MagickFalse)
                 break;
-
               if ((image->colors == 0) && (bpp != 24))
                 {
                   size_t
                     one;
 

Vulnerable code:         ThrowImageException(ResourceLimitError,"MemoryAllocationFailed");

Fixed code:       cache_info->type=UndefinedCache;
      cache_info->type=UndefinedCache;
      cache_info->type=UndefinedCache;
              if (status == MagickFalse)
                cache_info->type=UndefinedCache;
  if (status == MagickFalse)
    cache_info->type=UndefinedCache;
        {
          clone_image=DestroyImage(clone_image);
          ThrowImageException(ResourceLimitError,"MemoryAllocationFailed");
        }