From e9a386f8e00fa5a268dc87ee33764769e101b712 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastien=20ROUCARI=C3=88S?= <roucaries.bastien@gmail.com>
Date: Mon, 20 Feb 2012 14:52:22 +0100
Subject: [PATCH] ImageMagick Invalid Validation and Denial of Service

This patch fix two security bug
*    [CVE-2012-0247] When parsing a maliciously crafted image with incorrect offset and count in the ResolutionUnit tag in EXIF IFD0, ImageMagick copies two bytes into an invalid address.
*    [CVE-2012-0248] When parsing a maliciously crafted image with an IFD whose all IOP tags' value offsets point to the beginning of the IFD itself. As a result, ImageMagick parses the IFD structure indefinitely, causing a denial of service.

Thanks goes to the Mr Joonas Kuorilehto & Mr Aleksis Kauppinen from Codenomicon CROSS project for discovering the vulnerabilities and providing a test case file.
Also to the Finnish Communications Regulatory Authority (CERT-FI) for alerting us to these vulnerabilities.

(cherry picked from commit cb8dd0b021332238efa17bf88877723c8a960964)

Origin: upstream
Applied-Upstream: 6.7.5-0
Forwarded: http://www.imagemagick.org/discourse-server/viewtopic.php?f=4&t=20286
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=659339
---
 magick/property.c |  116 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 71 insertions(+), 45 deletions(-)

diff --git a/magick/property.c b/magick/property.c
index 7cf2c25..b8580b3 100644
--- a/magick/property.c
+++ b/magick/property.c
@@ -458,6 +458,13 @@ static inline long MagickMax(const long x,const long y)
   return(y);
 }
 
+static inline ssize_t MagickMin(const ssize_t x,const ssize_t y)
+{
+  if (x < y)
+    return(x);
+  return(y);
+}
+
 static inline int ReadPropertyByte(const unsigned char **p,size_t *length)
 {
   int
@@ -543,7 +550,6 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
     *info;
 
   long
-    id,
     start,
     stop,
     sub_number;
@@ -557,6 +563,8 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
   ssize_t
     count;
 
+  ssize_t id;
+
   size_t
     length;
 
@@ -592,10 +600,10 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
       continue;
     if (ReadPropertyByte(&info,&length) != (unsigned char) 'M')
       continue;
-    id=(long) ReadPropertyMSBShort(&info,&length);
-    if (id < start)
+    id=(ssize_t) ((int) ReadPropertyMSBShort(&info,&length));
+    if (id < (ssize_t) start)
       continue;
-    if (id > stop)
+    if (id > (ssize_t) stop)
       continue;
     if (resource != (char *) NULL)
       resource=DestroyString(resource);
@@ -623,7 +631,7 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
             No name match, scroll forward and try next.
           */
           info+=count;
-          length-=count;
+          length-=MagickMin(count,(ssize_t) length);
           continue;
         }
     if ((*name == '#') && (sub_number != 1))
@@ -633,7 +641,7 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
         */
         sub_number--;
         info+=count;
-        length-=count;
+        length-=MagickMin(count,(ssize_t) length);
         continue;
       }
     /*
@@ -648,7 +656,7 @@ static MagickBooleanType Get8BIMProperty(const Image *image,const char *key)
         (void) CopyMagickMemory(attribute,(char *) info,(size_t) count);
         attribute[count]='\0';
         info+=count;
-        length-=count;
+        length-=MagickMin(count,(ssize_t) length);
         if ((id <= 1999) || (id >= 2999))
           (void) SetImageProperty((Image *) image,key,(const char *)
             attribute);
@@ -1092,8 +1100,9 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
 
   long
     all,
-    id,
-    level,
+    level;
+
+  ssize_t 
     tag_value;
 
   register long
@@ -1102,6 +1111,11 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
   size_t
     length;
 
+  ssize_t id;
+
+  SplayTreeInfo
+    *exif_resources;
+
   ssize_t
     offset;
 
@@ -1109,10 +1123,12 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
     tag_bytes[] = {0, 1, 1, 2, 4, 8, 1, 1, 2, 4, 8, 4, 8};
 
   unsigned long
-    entry,
-    number_entries,
     tag_offset,
     tag;
+  
+  size_t number_entries;
+
+  size_t entry;
 
   /*
     If EXIF data exists, then try to parse the request for a tag.
@@ -1224,7 +1240,7 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
   }
   if (length < 16)
     return(MagickFalse);
-  id=(long) ReadPropertyShort(LSBEndian,exif);
+  id=(ssize_t) ((int) ReadPropertyShort(LSBEndian,exif));
   endian=LSBEndian;
   if (id == 0x4949)
     endian=LSBEndian;
@@ -1248,6 +1264,8 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
   level=0;
   entry=0;
   tag_offset=0;
+  exif_resources=NewSplayTree((int (*)(const void *,const void *)) NULL,
+    (void *(*)(void *)) NULL,(void *(*)(void *)) NULL);
   do
   {
     /*
@@ -1257,13 +1275,13 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
       {
         level--;
         directory=directory_stack[level].directory;
-        entry=directory_stack[level].entry;
+        entry=(size_t) directory_stack[level].entry;
         tag_offset=directory_stack[level].offset;
       }
     /*
       Determine how many entries there are in the current IFD.
     */
-    number_entries=ReadPropertyShort(endian,directory);
+    number_entries=(size_t) ((int) ReadPropertyShort(endian,directory));
     for ( ; entry < number_entries; entry++)
     {
       long
@@ -1276,12 +1294,15 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
       size_t
         number_bytes;
 
-      unsigned long
+      size_t
         format;
 
-      q=(unsigned char *) (directory+2+(12*entry));
-      tag_value=(long) ReadPropertyShort(endian,q)+tag_offset;
-      format=(unsigned long) ReadPropertyShort(endian,q+2);
+      q=(unsigned char *) (directory+(12*entry)+2);
+      if (GetValueFromSplayTree(exif_resources,q) == q)
+        break;
+      (void) AddValueToSplayTree(exif_resources,q,q);
+      tag_value=(ssize_t) ((int) ReadPropertyShort(endian,q)+tag_offset);
+      format=(size_t) ((int) ReadPropertyShort(endian,q+2));
       if (format >= (sizeof(tag_bytes)/sizeof(*tag_bytes)))
         break;
       components=(long) ReadPropertyLong(endian,q+4);
@@ -1333,7 +1354,8 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
             }
             case EXIF_FMT_ULONG:
             {
-              EXIFMultipleValues(4,"%lu",ReadPropertyLong(endian,p1));
+              EXIFMultipleValues(4,"%.20g",(double)
+                ((int) ReadPropertyLong(endian,p1)));
               break;
             }
             case EXIF_FMT_SLONG:
@@ -1343,8 +1365,9 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
             }
             case EXIF_FMT_URATIONAL:
             {
-              EXIFMultipleFractions(8,"%ld/%ld",ReadPropertyLong(endian,p1),
-                ReadPropertyLong(endian,p1+4));
+              EXIFMultipleFractions(8,"%.20g/%.20g",(double)
+                ((int) ReadPropertyLong(endian,p1)),(double)
+                ((int) ReadPropertyLong(endian,p1+4)));
               break;
             }
             case EXIF_FMT_SRATIONAL:
@@ -1410,7 +1433,7 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
                   {
                     if (EXIFTag[i].tag == 0)
                       break;
-                    if ((long) EXIFTag[i].tag == tag_value)
+                    if ((long) EXIFTag[i].tag == (long) tag_value)
                       {
                         description=EXIFTag[i].description;
                         break;
@@ -1450,7 +1473,7 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
             size_t
               offset;
 
-            offset=(size_t) ReadPropertyLong(endian,p);
+            offset=(size_t) ((int) ReadPropertyLong(endian,p));
             if ((offset < length) && (level < (MaxDirectoryStack-2)))
               {
                 unsigned long
@@ -1468,8 +1491,8 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
                 level++;
                 if ((directory+2+(12*number_entries)) > (exif+length))
                   break;
-                offset=(size_t) ReadPropertyLong(endian,directory+2+(12*
-                  number_entries));
+                offset=(size_t) ((int) ReadPropertyLong(endian,directory+2+(12*
+                  number_entries)));
                 if ((offset != 0) && (offset < length) &&
                     (level < (MaxDirectoryStack-2)))
                   {
@@ -1483,6 +1506,7 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
           }
     }
   } while (level > 0);
+  exif_resources=DestroySplayTree(exif_resources);
   return(MagickTrue);
 }
 
@@ -1572,10 +1596,12 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
     *message;
 
   long
-    knot_count,
-    selector,
     y;
 
+  ssize_t 
+    knot_count,
+    selector;
+
   MagickBooleanType
     in_subpath;
 
@@ -1584,7 +1610,7 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
     last[3],
     point[3];
 
-  register long
+  register ssize_t
     i,
     x;
 
@@ -1624,7 +1650,7 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
   in_subpath=MagickFalse;
   while (length > 0)
   {
-    selector=(long) ReadPropertyMSBShort(&blob,&length);
+    selector=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
     switch (selector)
     {
       case 0:
@@ -1633,15 +1659,15 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
         if (knot_count != 0)
           {
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
           Expected subpath length record.
         */
-        knot_count=(long) ReadPropertyMSBShort(&blob,&length);
+        knot_count=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
         blob+=22;
-        length-=22;
+        length-=MagickMin(22,(ssize_t) length);
         break;
       }
       case 1:
@@ -1655,7 +1681,7 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
               Unexpected subpath knot
             */
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
@@ -1663,13 +1689,13 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
         */
         for (i=0; i < 3; i++)
         {
-          unsigned long 
+          size_t
             xx,
             yy;
 
-          yy=ReadPropertyMSBLong(&blob,&length);
-          xx=ReadPropertyMSBLong(&blob,&length);
-          x=(long) xx;
+          yy=(size_t) ((int) ReadPropertyMSBLong(&blob,&length));
+          xx=(size_t) ((int) ReadPropertyMSBLong(&blob,&length));
+          x=(ssize_t) xx;
           if (xx > 2147483647)
             x=xx-4294967295-1;
           y=(long) yy;
@@ -1756,7 +1782,7 @@ static char *TracePSClippath(const unsigned char *blob,size_t length,
       default:
       {
         blob+=24;
-        length-=24;
+        length-=MagickMin(24,(ssize_t) length);
         break;
       }
     }
@@ -1821,7 +1847,7 @@ static char *TraceSVGClippath(const unsigned char *blob,size_t length,
   in_subpath=MagickFalse;
   while (length != 0)
   {
-    selector=(long) ReadPropertyMSBShort(&blob,&length);
+    selector=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
     switch (selector)
     {
       case 0:
@@ -1830,15 +1856,15 @@ static char *TraceSVGClippath(const unsigned char *blob,size_t length,
         if (knot_count != 0)
           {
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
           Expected subpath length record.
         */
-        knot_count=(long) ReadPropertyMSBShort(&blob,&length);
+        knot_count=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
         blob+=22;
-        length-=22;
+        length-=MagickMin(22,(ssize_t) length);
         break;
       }
       case 1:
@@ -1852,7 +1878,7 @@ static char *TraceSVGClippath(const unsigned char *blob,size_t length,
               Unexpected subpath knot.
             */
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
@@ -1860,7 +1886,7 @@ static char *TraceSVGClippath(const unsigned char *blob,size_t length,
         */
         for (i=0; i < 3; i++)
         {
-          unsigned long 
+          unsigned long
             xx,
             yy;
 
@@ -1927,7 +1953,7 @@ static char *TraceSVGClippath(const unsigned char *blob,size_t length,
       default:
       {
         blob+=24;
-        length-=24;
+        length-=MagickMin(24,(ssize_t) length);
         break;
       }
     }
-- 
1.7.10