Skip to content

Commit

Permalink
Improve checking of EXIF profile to prevent integer overflow (bug rep…
Browse files Browse the repository at this point in the history 
…ort from Ibrahim el-sayed)
  • Loading branch information
Cristy committed Jun 22, 2016
1 parent f0ae4ef commit d8ab7f0
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 49 deletions.
47 changes: 29 additions & 18 deletions MagickCore/profile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1351,20 +1351,22 @@ static inline const unsigned char *ReadResourceByte(const unsigned char *p,
static inline const unsigned char *ReadResourceLong(const unsigned char *p,
unsigned int *quantum)
{
*quantum=(size_t) (*p++ << 24);
*quantum|=(size_t) (*p++ << 16);
*quantum|=(size_t) (*p++ << 8);
*quantum|=(size_t) (*p++ << 0);
*quantum=(unsigned int) (*p++) << 24;
*quantum|=(unsigned int) (*p++) << 16;
*quantum|=(unsigned int) (*p++) << 8;
*quantum|=(unsigned int) (*p++) << 0;
return(p);
}

static inline const unsigned char *ReadResourceShort(const unsigned char *p,
unsigned short *quantum)
{
*quantum=(unsigned short) (*p++ << 8);
*quantum|=(unsigned short) (*p++ << 0);
*quantum=(unsigned short) (*p++) << 8;
*quantum|=(unsigned short) (*p++);
return(p);
}static inline void WriteResourceLong(unsigned char *p,
}

static inline void WriteResourceLong(unsigned char *p,
const unsigned int quantum)
{
unsigned char
Expand Down Expand Up @@ -1731,13 +1733,14 @@ static inline signed short ReadProfileShort(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned short) ((buffer[1] << 8) | buffer[0]);
quantum.unsigned_value=(value & 0xffff);
value=(unsigned short) buffer[1] << 8;
value|=(unsigned short) buffer[0];
quantum.unsigned_value=value & 0xffff;
return(quantum.signed_value);
}
value=(unsigned short) ((((unsigned char *) buffer)[0] << 8) |
((unsigned char *) buffer)[1]);
quantum.unsigned_value=(value & 0xffff);
value=(unsigned short) buffer[0] << 8;
value|=(unsigned short) buffer[1];
quantum.unsigned_value=value & 0xffff;
return(quantum.signed_value);
}

Expand All @@ -1758,14 +1761,18 @@ static inline signed int ReadProfileLong(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned int) ((buffer[3] << 24) | (buffer[2] << 16) |
(buffer[1] << 8 ) | (buffer[0]));
quantum.unsigned_value=(value & 0xffffffff);
value=(unsigned int) buffer[3] << 24;
value|=(unsigned int) buffer[2] << 16;
value|=(unsigned int) buffer[1] << 8;
value|=(unsigned int) buffer[0];
quantum.unsigned_value=value & 0xffffffff;
return(quantum.signed_value);
}
value=(unsigned int) ((buffer[0] << 24) | (buffer[1] << 16) |
(buffer[2] << 8) | buffer[3]);
quantum.unsigned_value=(value & 0xffffffff);
value=(unsigned int) buffer[0] << 24;
value|=(unsigned int) buffer[1] << 16;
value|=(unsigned int) buffer[2] << 8;
value|=(unsigned int) buffer[3];
quantum.unsigned_value=value & 0xffffffff;
return(quantum.signed_value);
}

Expand Down Expand Up @@ -2017,11 +2024,15 @@ MagickBooleanType SyncExifProfile(Image *image,StringInfo *profile)
tag_value;

q=(unsigned char *) (directory+2+(12*entry));
if (q > (exif+length-12))
break; /* corrupt EXIF */
tag_value=(ssize_t) ReadProfileShort(endian,q);
format=(ssize_t) ReadProfileShort(endian,q+2);
if ((format-1) >= EXIF_NUM_FORMATS)
break;
components=(ssize_t) ReadProfileLong(endian,q+4);
if (components < 0)
break; /* corrupt EXIF */
number_bytes=(size_t) components*format_bytes[format];
if ((ssize_t) number_bytes < components)
break; /* prevent overflow */
Expand Down
76 changes: 45 additions & 31 deletions MagickCore/property.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ static inline signed int ReadPropertyMSBLong(const unsigned char **p,
unsigned char
buffer[4];

size_t
unsigned int
value;

if (*length < 4)
Expand All @@ -531,11 +531,11 @@ static inline signed int ReadPropertyMSBLong(const unsigned char **p,
(*length)--;
buffer[i]=(unsigned char) c;
}
value=(size_t) (buffer[0] << 24);
value|=buffer[1] << 16;
value|=buffer[2] << 8;
value|=buffer[3];
quantum.unsigned_value=(value & 0xffffffff);
value=(unsigned int) buffer[0] << 24;
value|=(unsigned int) buffer[1] << 16;
value|=(unsigned int) buffer[2] << 8;
value|=(unsigned int) buffer[3];
quantum.unsigned_value=value & 0xffffffff;
return(quantum.signed_value);
}

Expand Down Expand Up @@ -571,9 +571,9 @@ static inline signed short ReadPropertyMSBShort(const unsigned char **p,
(*length)--;
buffer[i]=(unsigned char) c;
}
value=(unsigned short) (buffer[0] << 8);
value|=buffer[1];
quantum.unsigned_value=(value & 0xffff);
value=(unsigned short) buffer[0] << 8;
value|=(unsigned short) buffer[1];
quantum.unsigned_value=value & 0xffff;
return(quantum.signed_value);
}

Expand Down Expand Up @@ -742,14 +742,18 @@ static inline signed int ReadPropertySignedLong(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned int) ((buffer[3] << 24) | (buffer[2] << 16) |
(buffer[1] << 8 ) | (buffer[0]));
quantum.unsigned_value=(value & 0xffffffff);
value=(unsigned int) buffer[3] << 24;
value|=(unsigned int) buffer[2] << 16;
value|=(unsigned int) buffer[1] << 8;
value|=(unsigned int) buffer[0];
quantum.unsigned_value=value & 0xffffffff;
return(quantum.signed_value);
}
value=(unsigned int) ((buffer[0] << 24) | (buffer[1] << 16) |
(buffer[2] << 8) | buffer[3]);
quantum.unsigned_value=(value & 0xffffffff);
value=(unsigned int) buffer[0] << 24;
value|=(unsigned int) buffer[1] << 16;
value|=(unsigned int) buffer[2] << 8;
value|=(unsigned int) buffer[3];
quantum.unsigned_value=value & 0xffffffff;
return(quantum.signed_value);
}

Expand All @@ -761,13 +765,17 @@ static inline unsigned int ReadPropertyUnsignedLong(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned int) ((buffer[3] << 24) | (buffer[2] << 16) |
(buffer[1] << 8 ) | (buffer[0]));
return((unsigned int) (value & 0xffffffff));
value=(unsigned int) buffer[3] << 24;
value|=(unsigned int) buffer[2] << 16;
value|=(unsigned int) buffer[1] << 8;
value|=(unsigned int) buffer[0];
return(value & 0xffffffff);
}
value=(unsigned int) ((buffer[0] << 24) | (buffer[1] << 16) |
(buffer[2] << 8) | buffer[3]);
return((unsigned int) (value & 0xffffffff));
value=(unsigned int) buffer[0] << 24;
value|=(unsigned int) buffer[1] << 16;
value|=(unsigned int) buffer[2] << 8;
value|=(unsigned int) buffer[3];
return(value & 0xffffffff);
}

static inline signed short ReadPropertySignedShort(const EndianType endian,
Expand All @@ -787,13 +795,14 @@ static inline signed short ReadPropertySignedShort(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned short) ((buffer[1] << 8) | buffer[0]);
quantum.unsigned_value=(value & 0xffff);
value=(unsigned short) buffer[1] << 8;
value|=(unsigned short) buffer[0];
quantum.unsigned_value=value & 0xffff;
return(quantum.signed_value);
}
value=(unsigned short) ((((unsigned char *) buffer)[0] << 8) |
((unsigned char *) buffer)[1]);
quantum.unsigned_value=(value & 0xffff);
value=(unsigned short) buffer[0] << 8;
value|=(unsigned short) buffer[1];
quantum.unsigned_value=value & 0xffff;
return(quantum.signed_value);
}

Expand All @@ -805,12 +814,13 @@ static inline unsigned short ReadPropertyUnsignedShort(const EndianType endian,

if (endian == LSBEndian)
{
value=(unsigned short) ((buffer[1] << 8) | buffer[0]);
return((unsigned short) (value & 0xffff));
value=(unsigned short) buffer[1] << 8;
value|=(unsigned short) buffer[0];
return(value & 0xffff);
}
value=(unsigned short) ((((unsigned char *) buffer)[0] << 8) |
((unsigned char *) buffer)[1]);
return((unsigned short) (value & 0xffff));
value=(unsigned short) buffer[0] << 8;
value|=(unsigned short) buffer[1];
return(value & 0xffff);
}

static MagickBooleanType GetEXIFProperty(const Image *image,
Expand Down Expand Up @@ -1394,6 +1404,8 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
components;

q=(unsigned char *) (directory+(12*entry)+2);
if (q > (exif+length-12))
break; /* corrupt EXIF */
if (GetValueFromSplayTree(exif_resources,q) == q)
break;
(void) AddValueToSplayTree(exif_resources,q,q);
Expand All @@ -1402,6 +1414,8 @@ static MagickBooleanType GetEXIFProperty(const Image *image,
if (format >= (sizeof(tag_bytes)/sizeof(*tag_bytes)))
break;
components=(ssize_t) ReadPropertySignedLong(endian,q+4);
if (components < 0)
break; /* corrupt EXIF */
number_bytes=(size_t) components*tag_bytes[format];
if (number_bytes < components)
break; /* prevent overflow */
Expand Down

0 comments on commit d8ab7f0

Please sign in to comment.