ImageMagick Invalid Validation and Denial of Service

Announcements pertaining to ImageMagick, or ImageMagick related software. This list is moderated. No discussions here, instead post to the users group instead.
Post Reply
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

ImageMagick Invalid Validation and Denial of Service

Post by magick »

Concerning ImageMagick 6.7.5-7 and earlier:
  • [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/

Thanks goes to security @ redhat.com for alerting us that SyncImageProfiles() is vulnerable to CVE-2012-0248.

Here are patches that repairs these vulnerabilities:

Code: Select all

--- ImageMagick-6.7.4-10/magick/property.c      2011-12-23 15:52:11.000000000 -0500
+++ ImageMagick-6.7.5-8/magick/property.c       2012-02-29 09:55:22.305068822 -0500
@@ -443,6 +443,13 @@
   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
@@ -577,7 +584,7 @@
       continue;
     if (ReadPropertyByte(&info,&length) != (unsigned char) 'M')
       continue;
-    id=(ssize_t) ReadPropertyMSBShort(&info,&length);
+    id=(ssize_t) ((int) ReadPropertyMSBShort(&info,&length));
     if (id < (ssize_t) start)
       continue;
     if (id > (ssize_t) stop)
@@ -608,7 +615,7 @@
             No name match, scroll forward and try next.
           */
           info+=count;
-          length-=count;
+          length-=MagickMin(count,(ssize_t) length);
           continue;
         }
     if ((*name == '#') && (sub_number != 1))
@@ -618,7 +625,7 @@
         */
         sub_number--;
         info+=count;
-        length-=count;
+        length-=MagickMin(count,(ssize_t) length);
         continue;
       }
     /*
@@ -633,7 +640,7 @@
         (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);
@@ -773,7 +780,9 @@
       *directory;
 
     size_t
-      entry,
+      entry;
+
+    ssize_t
       offset;
   } DirectoryInfo;
 
@@ -1085,14 +1094,17 @@
     entry,
     length,
     number_entries,
-    tag_offset,
     tag;
 
+  SplayTreeInfo
+    *exif_resources;
+
   ssize_t
     all,
     id,
     level,
     offset,
+    tag_offset,
     tag_value;
 
   static int
@@ -1208,7 +1220,7 @@
   }
   if (length < 16)
     return(MagickFalse);
-  id=(ssize_t) ReadPropertyShort(LSBEndian,exif);
+  id=(ssize_t) ((int) ReadPropertyShort(LSBEndian,exif));
   endian=LSBEndian;
   if (id == 0x4949)
     endian=LSBEndian;
@@ -1223,7 +1235,7 @@
     This the offset to the first IFD.
   */
   offset=(ssize_t) ((int) ReadPropertyLong(endian,exif+4));
-  if ((size_t) offset >= length)
+  if ((offset < 0) || (size_t) offset >= length)
     return(MagickFalse);
   /*
     Set the pointer to the first IFD and follow it were it leads.
@@ -1233,6 +1245,8 @@
   level=0;
   entry=0;
   tag_offset=0;
+  exif_resources=NewSplayTree((int (*)(const void *,const void *)) NULL,
+    (void *(*)(void *)) NULL,(void *(*)(void *)) NULL);
   do
   {
     /*
@@ -1248,7 +1262,7 @@
     /*
       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++)
     {
       register unsigned char
@@ -1262,9 +1276,12 @@
       ssize_t
         components;
 
-      q=(unsigned char *) (directory+2+(12*entry));
-      tag_value=(ssize_t) (ReadPropertyShort(endian,q)+tag_offset);
-      format=(size_t) 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=(ssize_t) ((int) ReadPropertyLong(endian,q+4));
@@ -1280,6 +1297,8 @@
             The directory entry contains an offset.
           */
           offset=(ssize_t) ((int) ReadPropertyLong(endian,q+8));
+          if ((offset+number_bytes) < offset)
+            continue;  /* prevent overflow */
           if ((size_t) (offset+number_bytes) > length)
             continue;
           p=(unsigned char *) (exif+offset);
@@ -1316,7 +1335,7 @@
             case EXIF_FMT_ULONG:
             {
               EXIFMultipleValues(4,"%.20g",(double)
-                ReadPropertyLong(endian,p1));
+                ((int) ReadPropertyLong(endian,p1)));
               break;
             }
             case EXIF_FMT_SLONG:
@@ -1328,8 +1347,8 @@
             case EXIF_FMT_URATIONAL:
             {
               EXIFMultipleFractions(8,"%.20g/%.20g",(double)
-                ReadPropertyLong(endian,p1),(double)
-                ReadPropertyLong(endian,p1+4));
+                ((int) ReadPropertyLong(endian,p1)),(double)
+                ((int) ReadPropertyLong(endian,p1+4)));
               break;
             }
             case EXIF_FMT_SRATIONAL:
@@ -1430,19 +1449,19 @@
             }
         }
         if ((tag_value == TAG_EXIF_OFFSET) ||
-            (tag_value == TAG_INTEROP_OFFSET) ||
-            (tag_value == TAG_GPS_OFFSET))
+            (tag_value == TAG_INTEROP_OFFSET) || (tag_value == TAG_GPS_OFFSET))
           {
-            size_t
+            ssize_t
               offset;
 
-            offset=(size_t) ReadPropertyLong(endian,p);
-            if ((offset < length) && (level < (MaxDirectoryStack-2)))
+            offset=(ssize_t) ((int) ReadPropertyLong(endian,p));
+            if (((size_t) offset < length) && (level < (MaxDirectoryStack-2)))
               {
-                size_t
+                ssize_t
                   tag_offset1;
 
-                tag_offset1=(tag_value == TAG_GPS_OFFSET) ? 0x10000UL : 0UL;
+                tag_offset1=(ssize_t) ((tag_value == TAG_GPS_OFFSET) ? 0x10000 :
+                  0);
                 directory_stack[level].directory=directory;
                 entry++;
                 directory_stack[level].entry=entry;
@@ -1454,9 +1473,9 @@
                 level++;
                 if ((directory+2+(12*number_entries)) > (exif+length))
                   break;
-                offset=(size_t) ReadPropertyLong(endian,directory+2+(12*
-                  number_entries));
-                if ((offset != 0) && (offset < length) &&
+                offset=(ssize_t) ((int) ReadPropertyLong(endian,directory+2+(12*
+                  number_entries)));
+                if ((offset != 0) && ((size_t) offset < length) &&
                     (level < (MaxDirectoryStack-2)))
                   {
                     directory_stack[level].directory=exif+offset;
@@ -1469,6 +1488,7 @@
           }
     }
   } while (level > 0);
+  exif_resources=DestroySplayTree(exif_resources);
   return(status);
 }
 
@@ -1609,7 +1629,7 @@
   in_subpath=MagickFalse;
   while (length > 0)
   {
-    selector=(ssize_t) ReadPropertyMSBShort(&blob,&length);
+    selector=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
     switch (selector)
     {
       case 0:
@@ -1618,15 +1638,15 @@
         if (knot_count != 0)
           {
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
           Expected subpath length record.
         */
-        knot_count=(ssize_t) ReadPropertyMSBShort(&blob,&length);
+        knot_count=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
         blob+=22;
-        length-=22;
+        length-=MagickMin(22,(ssize_t) length);
         break;
       }
       case 1:
@@ -1640,7 +1660,7 @@
               Unexpected subpath knot
             */
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
@@ -1652,8 +1672,8 @@
             xx,
             yy;
 
-          yy=ReadPropertyMSBLong(&blob,&length);
-          xx=ReadPropertyMSBLong(&blob,&length);
+          yy=(size_t) ((int) ReadPropertyMSBLong(&blob,&length));
+          xx=(size_t) ((int) ReadPropertyMSBLong(&blob,&length));
           x=(ssize_t) xx;
           if (xx > 2147483647)
             x=(ssize_t) xx-4294967295U-1;
@@ -1741,7 +1761,7 @@
       default:
       {
         blob+=24;
-        length-=24;
+        length-=MagickMin(24,(ssize_t) length);
         break;
       }
     }
@@ -1806,7 +1826,7 @@
   in_subpath=MagickFalse;
   while (length != 0)
   {
-    selector=(ssize_t) ReadPropertyMSBShort(&blob,&length);
+    selector=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
     switch (selector)
     {
       case 0:
@@ -1815,15 +1835,15 @@
         if (knot_count != 0)
           {
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
           Expected subpath length record.
         */
-        knot_count=(ssize_t) ReadPropertyMSBShort(&blob,&length);
+        knot_count=(ssize_t) ((int) ReadPropertyMSBShort(&blob,&length));
         blob+=22;
-        length-=22;
+        length-=MagickMin(22,(ssize_t) length);
         break;
       }
       case 1:
@@ -1837,7 +1857,7 @@
               Unexpected subpath knot.
             */
             blob+=24;
-            length-=24;
+            length-=MagickMin(24,(ssize_t) length);
             break;
           }
         /*
@@ -1912,7 +1932,7 @@
       default:
       {
         blob+=24;
-        length-=24;
+        length-=MagickMin(24,(ssize_t) length);
         break;
       }
     }
--- ImageMagick-6.7.4-10/magick/profile.c       2011-12-23 12:03:02.000000000 -0500
+++ ImageMagick-6.7.5-8/magick/profile.c        2012-02-29 10:12:27.092282962 -0500
@@ -6620,17 +6620,18 @@
   EndianType
     endian;
 
-  int
-    offset;
-
   size_t
     entry,
     length,
     number_entries;
 
+  SplayTreeInfo
+    *exif_resources;
+
   ssize_t
     id,
-    level;
+    level,
+    offset;
 
   static int
     format_bytes[] = {0, 1, 1, 2, 4, 8, 1, 1, 2, 4, 8, 4, 8};
@@ -6682,12 +6683,14 @@
   /*
     This the offset to the first IFD.
   */
-  offset=(int) ReadProfileLong(endian,exif+4);
-  if ((size_t) offset >= length)
+  offset=(ssize_t) ((int) ReadProfileLong(endian,exif+4));
+  if ((offset < 0) || ((size_t) offset >= length))
     return(MagickFalse);
   directory=exif+offset;
   level=0;
   entry=0;
+  exif_resources=NewSplayTree((int (*)(const void *,const void *)) NULL,
+    (void *(*)(void *)) NULL,(void *(*)(void *)) NULL);
   do
   {
     if (level > 0)
@@ -6717,6 +6720,9 @@
         tag_value;
 
       q=(unsigned char *) (directory+2+(12*entry));
+      if (GetValueFromSplayTree(exif_resources,q) == q)
+        break;
+      (void) AddValueToSplayTree(exif_resources,q,q);
       tag_value=(ssize_t) ReadProfileShort(endian,q);
       format=(ssize_t) ReadProfileShort(endian,q+2);
       if ((format-1) >= EXIF_NUM_FORMATS)
@@ -6727,13 +6733,15 @@
         p=q+8;
       else
         {
-          int
+          ssize_t
             offset;
 
           /*
             The directory entry contains an offset.
           */
-          offset=(int) ReadProfileLong(endian,q+8);
+          offset=(ssize_t) ((int) ReadProfileLong(endian,q+8));
+          if ((offset+number_bytes) < offset)
+            continue;  /* prevent overflow */
           if ((size_t) (offset+number_bytes) > length)
             continue;
           p=(unsigned char *) (exif+offset);
@@ -6742,28 +6750,25 @@
       {
         case 0x011a:
         {
-          (void) WriteProfileLong(endian,(size_t)
-            (image->x_resolution+0.5),p);
+          (void) WriteProfileLong(endian,(size_t) (image->x_resolution+0.5),p);
           (void) WriteProfileLong(endian,1UL,p+4);
           break;
         }
         case 0x011b:
         {
-          (void) WriteProfileLong(endian,(size_t)
-            (image->y_resolution+0.5),p);
+          (void) WriteProfileLong(endian,(size_t) (image->y_resolution+0.5),p);
           (void) WriteProfileLong(endian,1UL,p+4);
           break;
         }
         case 0x0112:
         {
-          (void) WriteProfileShort(endian,(unsigned short)
-            image->orientation,p);
+          (void) WriteProfileShort(endian,(unsigned short) image->orientation,
+            p);
           break;
         }
         case 0x0128:
         {
-          (void) WriteProfileShort(endian,(unsigned short)
-            (image->units+1),p);
+          (void) WriteProfileShort(endian,(unsigned short) (image->units+1),p);
           break;
         }
         default:
@@ -6771,11 +6776,11 @@
       }
       if ((tag_value == TAG_EXIF_OFFSET) || (tag_value == TAG_INTEROP_OFFSET))
         {
-          size_t
+          ssize_t
             offset;
 
-          offset=(size_t) ReadProfileLong(endian,p);
-          if ((offset < length) && (level < (MaxDirectoryStack-2)))
+          offset=(ssize_t) ((int) ReadProfileLong(endian,p));
+          if (((size_t) offset < length) && (level < (MaxDirectoryStack-2)))
             {
               directory_stack[level].directory=directory;
               entry++;
@@ -6786,9 +6791,9 @@
               level++;
               if ((directory+2+(12*number_entries)) > (exif+length))
                 break;
-              offset=(size_t) ReadProfileLong(endian,directory+2+(12*
-                number_entries));
-              if ((offset != 0) && (offset < length) &&
+              offset=(ssize_t) ((int) ReadProfileLong(endian,directory+2+(12*
+                number_entries)));
+              if ((offset != 0) && ((size_t) offset < length) &&
                   (level < (MaxDirectoryStack-2)))
                 {
                   directory_stack[level].directory=exif+offset;
@@ -6800,5 +6805,6 @@
         }
     }
   } while (level > 0);
+  exif_resources=DestroySplayTree(exif_resources);
   return(MagickTrue);
 }
Post Reply