libexif: Fix out of bound reads in exif_mnote_data_olympus_load().

libexif/olympus/exif-mnote-data-olympus.c

 /* exif-mnote-data-olympus.c
  *
  * Copyright (c) 2002, 2003 Lutz Mueller <lutz@users.sourceforge.net>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 19 matching lines @@
 
 #define DEBUG
 
 /* Uncomment this to fix a problem with Sanyo MakerNotes. It's probably best
  * not to in most cases because it seems to only affect the thumbnail tag
  * which is duplicated in IFD 1, and fixing the offset could actually cause
  * problems with other software that expects the broken form.
  */
 /*#define EXIF_OVERCOME_SANYO_OFFSET_BUG */
 
+static int will_overflow(size_t num1, size_t num2)
+{
+        return num1 > (size_t)-1 - num2 ? 1 : 0;

[rickyz (no longer on Chrome), 2016/01/13 21:33:23]

Can this use SIZE_MAX from stdint.h instead, and is the ternary operator needed?

[Lei Zhang, 2016/01/13 22:02:19]

I'm hoping this is something that upstream will take. I'm not sure if SIZE_MAX
is always available.

The ternary operator is probably not needed.

[rickyz (no longer on Chrome), 2016/01/13 22:08:47]

Ah, didn't realize SIZE_MAX was only introduced in C99, makes sense.

+}
+
+static int exceeds_buffer(size_t buf_size, size_t index, size_t offset)

[rickyz (no longer on Chrome), 2016/01/13 21:33:23]

nit: Maybe the names (size_t buf_size, size_t offset, size_t len) make more
sense?

[Lei Zhang, 2016/01/13 22:02:19]

Done.

+{
+        return buf_size < offset || index > buf_size - offset ? 1 : 0;

[rickyz (no longer on Chrome), 2016/01/13 21:33:22]

Should the two comparisons be inclusive here?

[Lei Zhang, 2016/01/13 22:02:20]

I don't think so, exceeds_buffer(10, 0, 10) should return false, right?

[rickyz (no longer on Chrome), 2016/01/13 22:08:47]

Oops, never mind, I was confused by the argument names and thought the last two
arguments were indexes - this looks right.

+}
+
 static enum OlympusVersion
 exif_mnote_data_olympus_identify_variant (const unsigned char *buf,
                 unsigned int buf_size);
 
 
 static void
 exif_mnote_data_olympus_clear (ExifMnoteDataOlympus *n)
 {
         ExifMnoteData *d = (ExifMnoteData *) n;
         unsigned int i;
@@ skipping 184 matching lines @@
 }
 
 static void
 exif_mnote_data_olympus_load (ExifMnoteData *en,
                               const unsigned char *buf, unsigned int buf_size)
 {
         ExifMnoteDataOlympus *n = (ExifMnoteDataOlympus *) en;
         ExifShort c;
         size_t i, tcount, o, o2, datao = 6, base = 0;
 
-        if (!n || !buf || !buf_size) {
+        if (!n || !buf || !buf_size || will_overflow(n->offset, 6)) {
                 exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
                           "ExifMnoteDataOlympus", "Short MakerNote");
                 return;
         }
         o2 = 6 + n->offset; /* Start of interesting data */
-        if ((o2 + 10 < o2) || (o2 + 10 < 10) || (o2 + 10 > buf_size)) {
+        if (exceeds_buffer(buf_size, o2, 10)) {
                 exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
                           "ExifMnoteDataOlympus", "Short MakerNote");
                 return;
         }
 
         /*
          * Olympus headers start with "OLYMP" and need to have at least
          * a size of 22 bytes (6 for 'OLYMP', 2 other bytes, 2 for the
          * number of entries, and 12 for one entry.
          *
@@ skipping 20 matching lines @@
         case epsonV1:
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Parsing Olympus/Sanyo/Epson maker note v1...");
 
                 /* The number of entries is at position 8. */
                 if (buf[o2 + 6] == 1)
                         n->order = EXIF_BYTE_ORDER_INTEL;
                 else if (buf[o2 + 6 + 1] == 1)
                         n->order = EXIF_BYTE_ORDER_MOTOROLA;
                 o2 += 8;
-                if (o2 + 2 > buf_size) return;
                 c = exif_get_short (buf + o2, n->order);
                 if ((!(c & 0xFF)) && (c > 0x500)) {
                         if (n->order == EXIF_BYTE_ORDER_INTEL) {
                                 n->order = EXIF_BYTE_ORDER_MOTOROLA;
                         } else {
                                 n->order = EXIF_BYTE_ORDER_INTEL;
                         }
                 }
                 break;
 
         case olympusV2:
                 /* Olympus S760, S770 */
                 datao = o2;
                 o2 += 8;
+                if (exceeds_buffer(buf_size, o2, 4)) return;
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Parsing Olympus maker note v2 (0x%02x, %02x, %02x, %02x)...",
                         buf[o2], buf[o2 + 1], buf[o2 + 2], buf[o2 + 3]);
 
                 if ((buf[o2] == 'I') && (buf[o2 + 1] == 'I'))
                         n->order = EXIF_BYTE_ORDER_INTEL;
                 else if ((buf[o2] == 'M') && (buf[o2 + 1] == 'M'))
                         n->order = EXIF_BYTE_ORDER_MOTOROLA;
 
                 /* The number of entries is at position 8+4. */
+                if (will_overflow(o2, 4)) return;
                 o2 += 4;
                 break;
 
         case nikonV1:
                 o2 += 6;
-                if (o2 >= buf_size) return;
+                if (exceeds_buffer(buf_size, o2, 8)) return;
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Parsing Nikon maker note v1 (0x%02x, %02x, %02x, "
                         "%02x, %02x, %02x, %02x, %02x)...",
                         buf[o2 + 0], buf[o2 + 1], buf[o2 + 2], buf[o2 + 3], 
                         buf[o2 + 4], buf[o2 + 5], buf[o2 + 6], buf[o2 + 7]);
 
                 /* Skip version number */
                 o2 += 1;
 
                 /* Skip an unknown byte (00 or 0A). */
                 o2 += 1;
 
                 base = MNOTE_NIKON1_TAG_BASE;
                 /* Fix endianness, if needed */
-                if (o2 + 2 > buf_size) return;
                 c = exif_get_short (buf + o2, n->order);
                 if ((!(c & 0xFF)) && (c > 0x500)) {
                         if (n->order == EXIF_BYTE_ORDER_INTEL) {
                                 n->order = EXIF_BYTE_ORDER_MOTOROLA;
                         } else {
                                 n->order = EXIF_BYTE_ORDER_INTEL;
                         }
                 }
                 break;
 
         case nikonV2:
                 o2 += 6;
-                if (o2 >= buf_size) return;
+                if (exceeds_buffer(buf_size, o2, 8)) return;
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Parsing Nikon maker note v2 (0x%02x, %02x, %02x, "
                         "%02x, %02x, %02x, %02x, %02x)...",
                         buf[o2 + 0], buf[o2 + 1], buf[o2 + 2], buf[o2 + 3], 
                         buf[o2 + 4], buf[o2 + 5], buf[o2 + 6], buf[o2 + 7]);
 
                 /* Skip version number */
                 o2 += 1;
 
                 /* Skip an unknown byte (00 or 0A). */
                 o2 += 1;
 
                 /* Skip 2 unknown bytes (00 00). */
                 o2 += 2;
 
                 /*
                  * Byte order. From here the data offset
                  * gets calculated.
                  */
+                if (exceeds_buffer(buf_size, o2, 2)) return;
                 datao = o2;
-                if (o2 >= buf_size) return;
                 if (!strncmp ((char *)&buf[o2], "II", 2))
                         n->order = EXIF_BYTE_ORDER_INTEL;
                 else if (!strncmp ((char *)&buf[o2], "MM", 2))
                         n->order = EXIF_BYTE_ORDER_MOTOROLA;
                 else {
                         exif_log (en->log, EXIF_LOG_CODE_DEBUG,
                                 "ExifMnoteDataOlympus", "Unknown "
                                 "byte order '%c%c'", buf[o2],
                                 buf[o2 + 1]);
                         return;
                 }
                 o2 += 2;
 
                 /* Skip 2 unknown bytes (00 2A). */
+                if (will_overflow(o2, 2)) return;
                 o2 += 2;
 
                 /* Go to where the number of entries is. */
-                if (o2 + 4 > buf_size) return;
+                if (exceeds_buffer(buf_size, o2, 4)) return;
+                if (will_overflow(datao, exif_get_long (buf + o2, n->order))) return;
                 o2 = datao + exif_get_long (buf + o2, n->order);
                 break;
 
         case nikonV0:
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Parsing Nikon maker note v0 (0x%02x, %02x, %02x, "
                         "%02x, %02x, %02x, %02x, %02x)...",
                         buf[o2 + 0], buf[o2 + 1], buf[o2 + 2], buf[o2 + 3], 
                         buf[o2 + 4], buf[o2 + 5], buf[o2 + 6], buf[o2 + 7]);
                 /* 00 1b is # of entries in Motorola order - the rest should also be in MM order */
                 n->order = EXIF_BYTE_ORDER_MOTOROLA;
                 break;
         
         default:
                 exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
                         "Unknown Olympus variant %i.", n->version);
                 return;
         }
 
         /* Sanity check the offset */
-        if ((o2 + 2 < o2) || (o2 + 2 < 2) || (o2 + 2 > buf_size)) {
+        if (exceeds_buffer(buf_size, o2, 2)) {
                 exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
                           "ExifMnoteOlympus", "Short MakerNote");
                 return;
         }
 
         /* Read the number of tags */
         c = exif_get_short (buf + o2, n->order);
         o2 += 2;
 
         /* Remove any old entries */
         exif_mnote_data_olympus_clear (n);
 
         /* Reserve enough space for all the possible MakerNote tags */
         n->entries = exif_mem_alloc (en->mem, sizeof (MnoteOlympusEntry) * c);
         if (!n->entries) {
                 EXIF_LOG_NO_MEMORY(en->log, "ExifMnoteOlympus", sizeof (MnoteOlympusEntry) * c);
                 return;
         }
 
         /* Parse all c entries, storing ones that are successfully parsed */
         tcount = 0;
         for (i = c, o = o2; i; --i, o += 12) {
                 size_t s;
-                if ((o + 12 < o) || (o + 12 < 12) || (o + 12 > buf_size)) {
+                if (exceeds_buffer(buf_size, o, 12)) {
                         exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
                                   "ExifMnoteOlympus", "Short MakerNote");
                         break;
                 }
 
             n->entries[tcount].tag        = exif_get_short (buf + o, n->order) + base;
             n->entries[tcount].format     = exif_get_short (buf + o + 2, n->order);
             n->entries[tcount].components = exif_get_long (buf + o + 4, n->order);
             n->entries[tcount].order      = n->order;
 
@@ skipping 27 matching lines @@
                                  */
                             if (dataofs + s > buf_size && n->version == sanyoV1) {
                                         /* fix pointer */
                                         dataofs -= datao + 6;
                                         exif_log (en->log, EXIF_LOG_CODE_DEBUG,
                                                   "ExifMnoteOlympus",
                                                   "Inconsistent thumbnail tag offset; attempting to recover");
                             }
 #endif
                         }
-                        if ((dataofs + s < dataofs) || (dataofs + s < s) || 
-                            (dataofs + s > buf_size)) {
+                        if (exceeds_buffer(buf_size, dataofs, s)) {
                                 exif_log (en->log, EXIF_LOG_CODE_DEBUG,
                                           "ExifMnoteOlympus",
                                           "Tag data past end of buffer (%u > %u)",
                                           dataofs + s, buf_size);
                                 continue;
                         }
 
                         n->entries[tcount].data = exif_mem_alloc (en->mem, s);
                         if (!n->entries[tcount].data) {
                                 EXIF_LOG_NO_MEMORY(en->log, "ExifMnoteOlympus", s);
@@ skipping 157 matching lines @@
         d->methods.save            = exif_mnote_data_olympus_save;
         d->methods.count           = exif_mnote_data_olympus_count;
         d->methods.get_id          = exif_mnote_data_olympus_get_id;
         d->methods.get_name        = exif_mnote_data_olympus_get_name;
         d->methods.get_title       = exif_mnote_data_olympus_get_title;
         d->methods.get_description = exif_mnote_data_olympus_get_description;
         d->methods.get_value       = exif_mnote_data_olympus_get_value;
 
         return d;
 }