Skip to content
Permalink
Browse files Browse the repository at this point in the history
avoid the use of unsafe integer overflow checking constructs (unsigned integer operations cannot overflow, so "u1 + u2 > u1" can be optimized away)

check for the actual sizes, which should also handle the overflows
document other places google patched, but do not seem relevant due to other restrictions

fixes #26
  • Loading branch information
msmeissn committed Jan 18, 2020
1 parent da025b3 commit 75aa732
Showing 1 changed file with 18 additions and 10 deletions.
28 changes: 18 additions & 10 deletions libexif/exif-data.c
Expand Up @@ -192,9 +192,15 @@ exif_data_load_data_entry (ExifData *data, ExifEntry *entry,
doff = offset + 8;

/* Sanity checks */
if ((doff + s < doff) || (doff + s < s) || (doff + s > size)) {
if (doff >= size) {
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
"Tag data past end of buffer (%u > %u)", doff+s, size);
"Tag starts past end of buffer (%u > %u)", doff, size);
return 0;
}

if (s > size - doff) {
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
"Tag data goes past end of buffer (%u > %u)", doff+s, size);
return 0;
}

Expand Down Expand Up @@ -315,13 +321,14 @@ exif_data_load_data_thumbnail (ExifData *data, const unsigned char *d,
unsigned int ds, ExifLong o, ExifLong s)
{
/* Sanity checks */
if ((o + s < o) || (o + s < s) || (o + s > ds) || (o > ds)) {
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
"Bogus thumbnail offset (%u) or size (%u).",
o, s);
if (o >= ds) {
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData", "Bogus thumbnail offset (%u).", o);
return;
}
if (s > ds - o) {
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData", "Bogus thumbnail size (%u), max would be %u.", s, ds-o);
return;
}

if (data->data)
exif_mem_free (data->priv->mem, data->data);
if (!(data->data = exif_data_alloc (data, s))) {
Expand Down Expand Up @@ -947,7 +954,7 @@ exif_data_load_data (ExifData *data, const unsigned char *d_orig,
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
"IFD 0 at %i.", (int) offset);

/* Sanity check the offset, being careful about overflow */
/* ds is restricted to 16 bit above, so offset is restricted too, and offset+8 should not overflow. */
if (offset > ds || offset + 6 + 2 > ds)
return;

Expand All @@ -956,6 +963,7 @@ exif_data_load_data (ExifData *data, const unsigned char *d_orig,

/* IFD 1 offset */
n = exif_get_short (d + 6 + offset, data->priv->order);
/* offset < 2<<16, n is 16 bit at most, so this op will not overflow */
if (offset + 6 + 2 + 12 * n + 4 > ds)
return;

Expand All @@ -964,8 +972,8 @@ exif_data_load_data (ExifData *data, const unsigned char *d_orig,
exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
"IFD 1 at %i.", (int) offset);

/* Sanity check. */
if (offset > ds || offset + 6 > ds) {
/* Sanity check. ds is ensured to be above 6 above, offset is 16bit */
if (offset > ds - 6) {
exif_log (data->priv->log, EXIF_LOG_CODE_CORRUPT_DATA,
"ExifData", "Bogus offset of IFD1.");
} else {
Expand Down

1 comment on commit 75aa732

@kloczek
Copy link

@kloczek kloczek commented on 75aa732 Feb 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any plans to make new release after that patch?

Please sign in to comment.