ImageMagick Invalid Validation and Denial of Service
Posted: 2012-02-03T19:08:45-07:00
Concerning ImageMagick 6.7.5-7 and earlier:
Thanks goes to security @ redhat.com for alerting us that SyncImageProfiles() is vulnerable to CVE-2012-0248.
Here are patches that repairs these vulnerabilities:
- [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 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);
}