Fix MakerNote tag size overflow issues at read time. am: fd5f7bab83 am: d303368591 am: aa9a617a21

Original change: https://googleplex-android-review.googlesource.com/c/platform/external/libexif/+/15718931

Change-Id: Ibd223e059b67bcc954704123ad7ce5dbb47e091a

4 files changed, 69 insertions, 19 deletions

diff --git a/libexif/canon/exif-mnote-data-canon.c b/libexif/canon/exif-mnote-data-canon.c
index acf88ab..4396c53 100644
--- a/libexif/canon/exif-mnote-data-canon.c
+++ b/libexif/canon/exif-mnote-data-canon.c
@@ -32,6 +32,8 @@
 
 #define DEBUG
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 static void
 exif_mnote_data_canon_clear (ExifMnoteDataCanon *n)
 {
@@ -209,7 +211,8 @@ exif_mnote_data_canon_load (ExifMnoteData *ne,
 		return;
 	}
 	datao = 6 + n->offset;
-	if ((datao + 2 < datao) || (datao + 2 < 2) || (datao + 2 > buf_size)) {
+
+	if (CHECKOVERFLOW(datao, buf_size, 2)) {
 		exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteCanon", "Short MakerNote");
 		return;
@@ -233,7 +236,7 @@ exif_mnote_data_canon_load (ExifMnoteData *ne,
 	tcount = 0;
 	for (i = c, o = datao; i; --i, o += 12) {
 		size_t s;
-		if ((o + 12 < o) || (o + 12 < 12) || (o + 12 > buf_size)) {
+		if (CHECKOVERFLOW(o,buf_size,12)) {
 			exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
 				"ExifMnoteCanon", "Short MakerNote");
 			break;
@@ -248,6 +251,16 @@ exif_mnote_data_canon_load (ExifMnoteData *ne,
 			"Loading entry 0x%x ('%s')...", n->entries[tcount].tag,
 			 mnote_canon_tag_get_name (n->entries[tcount].tag));
 
+		/* Check if we overflow the multiplication. Use buf_size as the max size for integer overflow detection,
+		 * we will check the buffer sizes closer later. */
+		if (	exif_format_get_size (n->entries[tcount].format) &&
+			buf_size / exif_format_get_size (n->entries[tcount].format) < n->entries[tcount].components
+		) {
+			exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
+				  "ExifMnoteCanon", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[tcount].format), n->entries[tcount].components);
+			continue;
+		}
+
 		/*
 		 * Size? If bigger than 4 bytes, the actual data is not
 		 * in the entry but somewhere else (offset).
@@ -264,7 +277,8 @@ exif_mnote_data_canon_load (ExifMnoteData *ne,
 		} else {
 			size_t dataofs = o + 8;
 			if (s > 4) dataofs = exif_get_long (buf + dataofs, n->order) + 6;
-			if ((dataofs + s < s) || (dataofs + s < dataofs) || (dataofs + s > buf_size)) {
+
+			if (CHECKOVERFLOW(dataofs, buf_size, s)) {
 				exif_log (ne->log, EXIF_LOG_CODE_DEBUG,
 					"ExifMnoteCanon",
 					"Tag data past end of buffer (%zu > %u)",
diff --git a/libexif/fuji/exif-mnote-data-fuji.c b/libexif/fuji/exif-mnote-data-fuji.c
index a9949e1..11ff8c3 100644
--- a/libexif/fuji/exif-mnote-data-fuji.c
+++ b/libexif/fuji/exif-mnote-data-fuji.c
@@ -28,6 +28,8 @@
 
 #include "exif-mnote-data-fuji.h"
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 struct _MNoteFujiDataPrivate {
 	ExifByteOrder order;
 };
@@ -162,7 +164,7 @@ exif_mnote_data_fuji_load (ExifMnoteData *en,
 		return;
 	}
 	datao = 6 + n->offset;
-	if ((datao + 12 < datao) || (datao + 12 < 12) || (datao + 12 > buf_size)) {
+	if (CHECKOVERFLOW(datao, buf_size, 12)) {
 		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteDataFuji", "Short MakerNote");
 		return;
@@ -170,8 +172,7 @@ exif_mnote_data_fuji_load (ExifMnoteData *en,
 
 	n->order = EXIF_BYTE_ORDER_INTEL;
 	datao += exif_get_long (buf + datao + 8, EXIF_BYTE_ORDER_INTEL);
-	if ((datao + 2 < datao) || (datao + 2 < 2) ||
-	    (datao + 2 > buf_size)) {
+	if (CHECKOVERFLOW(datao, buf_size, 2)) {
 		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteDataFuji", "Short MakerNote");
 		return;
@@ -195,7 +196,7 @@ exif_mnote_data_fuji_load (ExifMnoteData *en,
 	tcount = 0;
 	for (i = c, o = datao; i; --i, o += 12) {
 		size_t s;
-		if ((o + 12 < o) || (o + 12 < 12) || (o + 12 > buf_size)) {
+		if (CHECKOVERFLOW(o, buf_size, 12)) {
 			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 				  "ExifMnoteDataFuji", "Short MakerNote");
 			break;
@@ -210,6 +211,16 @@ exif_mnote_data_fuji_load (ExifMnoteData *en,
 			  "Loading entry 0x%x ('%s')...", n->entries[tcount].tag,
 			  mnote_fuji_tag_get_name (n->entries[tcount].tag));
 
+		/* Check if we overflow the multiplication. Use buf_size as the max size for integer overflow detection,
+		 * we will check the buffer sizes closer later. */
+		if (	exif_format_get_size (n->entries[tcount].format) &&
+			buf_size / exif_format_get_size (n->entries[tcount].format) < n->entries[tcount].components
+		) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+					  "ExifMnoteDataFuji", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[tcount].format), n->entries[tcount].components);
+			continue;
+		}
+
 		/*
 		 * Size? If bigger than 4 bytes, the actual data is not
 		 * in the entry but somewhere else (offset).
@@ -221,8 +232,7 @@ exif_mnote_data_fuji_load (ExifMnoteData *en,
 			if (s > 4)
 				/* The data in this case is merely a pointer */
 				dataofs = exif_get_long (buf + dataofs, n->order) + 6 + n->offset;
-			if ((dataofs + s < dataofs) || (dataofs + s < s) ||
-				(dataofs + s >= buf_size)) {
+			if (CHECKOVERFLOW(dataofs, buf_size, s)) {
 				exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 						  "ExifMnoteDataFuji", "Tag data past end of "
 					  "buffer (%zu >= %u)", dataofs + s, buf_size);
diff --git a/libexif/olympus/exif-mnote-data-olympus.c b/libexif/olympus/exif-mnote-data-olympus.c
index f4ccbb0..e7bf984 100644
--- a/libexif/olympus/exif-mnote-data-olympus.c
+++ b/libexif/olympus/exif-mnote-data-olympus.c
@@ -37,6 +37,8 @@
  */
 /*#define EXIF_OVERCOME_SANYO_OFFSET_BUG */
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 static enum OlympusVersion
 exif_mnote_data_olympus_identify_variant (const unsigned char *buf,
 		unsigned int buf_size);
@@ -247,7 +249,8 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 		return;
 	}
 	o2 = 6 + n->offset; /* Start of interesting data */
-	if ((o2 + 10 < o2) || (o2 + 10 < 10) || (o2 + 10 > buf_size)) {
+
+	if (CHECKOVERFLOW(o2,buf_size,10)) {
 		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteDataOlympus", "Short MakerNote");
 		return;
@@ -303,6 +306,8 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 		/* Olympus S760, S770 */
 		datao = o2;
 		o2 += 8;
+
+		if (CHECKOVERFLOW(o2,buf_size,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]);
@@ -347,6 +352,7 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 	case nikonV2:
 		o2 += 6;
 		if (o2 >= buf_size) return;
+		if (CHECKOVERFLOW(o2,buf_size,12)) return;
 		exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
 			"Parsing Nikon maker note v2 (0x%02x, %02x, %02x, "
 			"%02x, %02x, %02x, %02x, %02x)...",
@@ -406,7 +412,8 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 	}
 
 	/* Sanity check the offset */
-	if ((o2 + 2 < o2) || (o2 + 2 < 2) || (o2 + 2 > buf_size)) {
+
+	if (CHECKOVERFLOW(o2,buf_size,2)) {
 		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteOlympus", "Short MakerNote");
 		return;
@@ -430,7 +437,7 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 	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 (CHECKOVERFLOW(o, buf_size, 12)) {
 			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 				  "ExifMnoteOlympus", "Short MakerNote");
 			break;
@@ -451,6 +458,15 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 		    n->entries[tcount].components,
 		    (int)exif_format_get_size(n->entries[tcount].format)); */
 
+	    /* Check if we overflow the multiplication. Use buf_size as the max size for integer overflow detection,
+	     * we will check the buffer sizes closer later. */
+	    if (exif_format_get_size (n->entries[tcount].format) &&
+		buf_size / exif_format_get_size (n->entries[tcount].format) < n->entries[tcount].components
+	    ) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteOlympus", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[tcount].format), n->entries[tcount].components);
+		continue;
+	    }
+
 	    /*
 	     * Size? If bigger than 4 bytes, the actual data is not
 	     * in the entry but somewhere else (offset).
@@ -469,7 +485,7 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 				 * tag in its MakerNote. The offset is actually the absolute
 				 * position in the file instead of the position within the IFD.
 				 */
-			    if (dataofs + s > buf_size && n->version == sanyoV1) {
+			    if (dataofs > (buf_size - s) && n->version == sanyoV1) {
 					/* fix pointer */
 					dataofs -= datao + 6;
 					exif_log (en->log, EXIF_LOG_CODE_DEBUG,
@@ -478,8 +494,7 @@ exif_mnote_data_olympus_load (ExifMnoteData *en,
 			    }
 #endif
 			}
-			if ((dataofs + s < dataofs) || (dataofs + s < s) || 
-			    (dataofs + s > buf_size)) {
+			if (CHECKOVERFLOW(dataofs, buf_size, s)) {
 				exif_log (en->log, EXIF_LOG_CODE_DEBUG,
 					  "ExifMnoteOlympus",
 					  "Tag data past end of buffer (%zu > %u)",
diff --git a/libexif/pentax/exif-mnote-data-pentax.c b/libexif/pentax/exif-mnote-data-pentax.c
index 38fbf64..f9eb69c 100644
--- a/libexif/pentax/exif-mnote-data-pentax.c
+++ b/libexif/pentax/exif-mnote-data-pentax.c
@@ -28,6 +28,8 @@
 #include <libexif/exif-byte-order.h>
 #include <libexif/exif-utils.h>
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 static void
 exif_mnote_data_pentax_clear (ExifMnoteDataPentax *n)
 {
@@ -224,7 +226,7 @@ exif_mnote_data_pentax_load (ExifMnoteData *en,
 		return;
 	}
 	datao = 6 + n->offset;
-	if ((datao + 8 < datao) || (datao + 8 < 8) || (datao + 8 > buf_size)) {
+	if (CHECKOVERFLOW(datao, buf_size, 8)) {
 		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 			  "ExifMnoteDataPentax", "Short MakerNote");
 		return;
@@ -277,7 +279,7 @@ exif_mnote_data_pentax_load (ExifMnoteData *en,
 	tcount = 0;
 	for (i = c, o = datao; i; --i, o += 12) {
 		size_t s;
-		if ((o + 12 < o) || (o + 12 < 12) || (o + 12 > buf_size)) {
+		if (CHECKOVERFLOW(o,buf_size,12)) {
 			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
 				  "ExifMnoteDataPentax", "Short MakerNote");
 			break;
@@ -292,6 +294,16 @@ exif_mnote_data_pentax_load (ExifMnoteData *en,
 			  "Loading entry 0x%x ('%s')...", n->entries[tcount].tag,
 			  mnote_pentax_tag_get_name (n->entries[tcount].tag));
 
+		/* Check if we overflow the multiplication. Use buf_size as the max size for integer overflow detection,
+		 * we will check the buffer sizes closer later. */
+		if (	exif_format_get_size (n->entries[tcount].format) &&
+			buf_size / exif_format_get_size (n->entries[tcount].format) < n->entries[tcount].components
+		) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+				  "ExifMnoteDataPentax", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[tcount].format), n->entries[tcount].components);
+			break;
+		}
+
 		/*
 		 * Size? If bigger than 4 bytes, the actual data is not
 		 * in the entry but somewhere else (offset).
@@ -304,8 +316,7 @@ exif_mnote_data_pentax_load (ExifMnoteData *en,
 			if (s > 4)
 				/* The data in this case is merely a pointer */
 			   	dataofs = exif_get_long (buf + dataofs, n->order) + 6;
-			if ((dataofs + s < dataofs) || (dataofs + s < s) ||
-				(dataofs + s > buf_size)) {
+			if (CHECKOVERFLOW(dataofs, buf_size, s)) {
 				exif_log (en->log, EXIF_LOG_CODE_DEBUG,
 						  "ExifMnoteDataPentax", "Tag data past end "
 					  "of buffer (%zu > %u)", dataofs + s, buf_size);