From 435e21f05001fb03f9f186fa7cbc69454afd00d1 Mon Sep 17 00:00:00 2001
From: Dan Fandrich <dan@coneharvesters.com>
Date: Sat, 16 May 2020 17:32:28 +0200
Subject: [PATCH] Fix MakerNote tag size overflow issues at read time.

Check for a size overflow while reading tags, which ensures that the
size is always consistent for the given components and type of the
entry, making checking further down superfluous.

This provides an alternate fix for
https://sourceforge.net/p/libexif/bugs/125/ CVE-2016-6328 and for all
the MakerNote types. Likely, this makes both commits 41bd0423 and
89e5b1c1 redundant as it ensures that MakerNote entries are well-formed
when they're populated.

Some improvements on top by Marcus Meissner <marcus@jet.franken.de>

CVE-2020-13112
---
 libexif/canon/exif-mnote-data-canon.c     | 22 ++++++++++++++++----
 libexif/fuji/exif-mnote-data-fuji.c       | 24 ++++++++++++++++------
 libexif/olympus/exif-mnote-data-olympus.c | 25 +++++++++++++++--------
 libexif/pentax/exif-mnote-data-pentax.c   | 20 ++++++++++++++----
 4 files changed, 69 insertions(+), 22 deletions(-)

Index: libexif-0.6.17/libexif/canon/exif-mnote-data-canon.c
===================================================================
--- libexif-0.6.17.orig/libexif/canon/exif-mnote-data-canon.c
+++ libexif-0.6.17/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)
 {
@@ -198,7 +200,18 @@ exif_mnote_data_canon_load (ExifMnoteDat
 	size_t i, o, s;
 	MnoteCanonEntry *t;
 
-	if (!n || !buf || !buf_size || (buf_size < 6 + n->offset + 2)) return;
+	if (!n || !buf || !buf_size) {
+		exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
+		"ExifMnoteCanon", "Short MakerNote");
+		return;
+	}
+
+	if (CHECKOVERFLOW(n->offset, buf_size, 8)) {
+		exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
+		"ExifMnoteCanon", "Short MakerNote");
+		return;
+	}
+
 
 	/* Read the number of entries and remove old ones. */
 	c = exif_get_short (buf + 6 + n->offset, n->order);
@@ -207,7 +220,11 @@ exif_mnote_data_canon_load (ExifMnoteDat
 	/* Parse the entries */
 	for (i = 0; i < c; i++) {
 		o = 6 + 2 + n->offset + 12 * i;
-	  if (o + 8 > buf_size) return;
+
+		if (CHECKOVERFLOW(o,buf_size,12)) {
+			exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteCanon", "Short MakerNote");
+			break;
+		}
 
 		t = exif_mem_realloc (ne->mem, n->entries,
 				sizeof (MnoteCanonEntry) * (i + 1));
@@ -224,11 +241,24 @@ exif_mnote_data_canon_load (ExifMnoteDat
 	   * Size? If bigger than 4 bytes, the actual data is not
 	   * in the entry but somewhere else (offset).
 	   */
+		/* 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[i].format) &&
+			buf_size / exif_format_get_size (n->entries[i].format) < n->entries[i].components
+		) {
+			exif_log (ne->log, EXIF_LOG_CODE_CORRUPT_DATA,
+				  "ExifMnoteCanon", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[i].format), n->entries[i].components);
+			continue;
+		}
+
 	  s = exif_format_get_size (n->entries[i].format) * n->entries[i].components;
 		if (!s) return;
 		o += 8;
 		if (s > 4) o = exif_get_long (buf + o, n->order) + 6;
-		if (o + s > buf_size) return;
+		if (CHECKOVERFLOW(o, buf_size, s)) {
+			exif_log (ne->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteCanon", "Tag data past end of buffer (%u > %u)", (unsigned)(o + s), buf_size);
+			continue;
+		}
 
 		/* Sanity check */
 		n->entries[i].data = exif_mem_alloc (ne->mem, sizeof (char) * s);
Index: libexif-0.6.17/libexif/fuji/exif-mnote-data-fuji.c
===================================================================
--- libexif-0.6.17.orig/libexif/fuji/exif-mnote-data-fuji.c
+++ libexif-0.6.17/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;
 };
@@ -155,11 +157,25 @@ exif_mnote_data_fuji_load (ExifMnoteData
 	size_t i, o, s, datao = 6 + n->offset;
 	MnoteFujiEntry *t;
 
-	if (!n || !buf || !buf_size || (buf_size < datao + 12)) return;
+	if (!n || !buf || !buf_size) return;
+
+	if (CHECKOVERFLOW(n->offset, buf_size, 6+8+4)) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+			"ExifMnoteDataFuji", "Short MakerNote");
+		return;
+	}
+
 
 	/* Read the number of entries and remove old ones. */
 	n->order = EXIF_BYTE_ORDER_INTEL;
 	datao += exif_get_long (buf + datao + 8, EXIF_BYTE_ORDER_INTEL);
+	if (CHECKOVERFLOW(datao, buf_size, 2)) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+		"ExifMnoteDataFuji", "Short MakerNote");
+		return;
+	}
+
+
 	c = exif_get_short (buf + datao, EXIF_BYTE_ORDER_INTEL);
 	datao += 2;
 	exif_mnote_data_fuji_clear (n);
@@ -167,6 +183,11 @@ exif_mnote_data_fuji_load (ExifMnoteData
 	/* Parse the entries */
 	for (i = 0; i < c; i++) {
 		o = datao + 12 * i;
+		if (CHECKOVERFLOW(o, buf_size, 12)) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteDataFuji", "Short MakerNote");
+			break;
+		}
+
 		if (datao + 12 > buf_size) return;
 
 		t = exif_mem_realloc (en->mem, n->entries,
@@ -180,6 +201,15 @@ exif_mnote_data_fuji_load (ExifMnoteData
 		n->entries[i].components = exif_get_long (buf + o + 4, n->order);
 		n->entries[i].order      = n->order;
 
+		/* 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[i].format) &&
+			buf_size / exif_format_get_size (n->entries[i].format) < n->entries[i].components
+		) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+					  "ExifMnoteDataFuji", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[i].format), n->entries[i].components);
+			continue;
+		}
 		/*
 		 * Size? If bigger than 4 bytes, the actual data is not
 		 * in the entry but somewhere else (offset).
@@ -188,7 +218,10 @@ exif_mnote_data_fuji_load (ExifMnoteData
 		if (!s) return;
 		o += 8;
 		if (s > 4) o = exif_get_long (buf + o, n->order) + 6 + n->offset;
-		if (o + s > buf_size) return;
+		if (CHECKOVERFLOW(o, buf_size, s)) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteDataFuji", "Tag data past end of buffer (%u >= %u)", (unsigned)(o + s), buf_size);
+			continue;
+		}
 
 		/* Sanity check */
 		n->entries[i].data = exif_mem_alloc (en->mem, s);
Index: libexif-0.6.17/libexif/olympus/exif-mnote-data-olympus.c
===================================================================
--- libexif-0.6.17.orig/libexif/olympus/exif-mnote-data-olympus.c
+++ libexif-0.6.17/libexif/olympus/exif-mnote-data-olympus.c
@@ -28,6 +28,8 @@
 #include <libexif/exif-utils.h>
 #include <libexif/exif-data.h>
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 #define DEBUG
 
 static void
@@ -211,8 +213,12 @@ exif_mnote_data_olympus_load (ExifMnoteD
 
 	if (!n || !buf) return;
 
-	/* Start of interesting data */
-	o2 = 6 + n->offset;
+	o2 = 6 + n->offset; /* Start of interesting data */
+	if (CHECKOVERFLOW(o2,buf_size,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
@@ -261,6 +267,9 @@ exif_mnote_data_olympus_load (ExifMnoteD
 		/* 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]);
@@ -276,6 +285,9 @@ exif_mnote_data_olympus_load (ExifMnoteD
 
 	} else if (!memcmp (buf + o2, "Nikon", 6)) {
 		o2 += 6;
+
+		if (CHECKOVERFLOW(o2,buf_size,12)) return;
+
 		exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataOlympus",
 			"Parsing Nikon maker note (0x%02x, %02x, %02x, "
 			"%02x, %02x, %02x, %02x, %02x)...",
@@ -351,8 +363,14 @@ exif_mnote_data_olympus_load (ExifMnoteD
 		return;
 	}
 
+
+	if (CHECKOVERFLOW(o2,buf_size,2)) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+       		"ExifMnoteOlympus", "Short MakerNote");
+		return;
+	}
+
 	/* Number of entries */
-	if (o2 >= buf_size) return;
 	c = exif_get_short (buf + o2, n->order);
 	o2 += 2;
 
@@ -365,8 +383,10 @@ exif_mnote_data_olympus_load (ExifMnoteD
 	/* Parse the entries */
 	for (i = 0; i < c; i++) {
 	    o = o2 + 12 * i;
-	    if (o + 12 > buf_size) return;
-
+	    if (CHECKOVERFLOW(o, buf_size, 12)) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteOlympus", "Short MakerNote");
+		break;
+	    }
 	    n->count = i + 1;
 	    n->entries[i].tag        = exif_get_short (buf + o, n->order) + base;
 	    n->entries[i].format     = exif_get_short (buf + o + 2, n->order);
@@ -377,6 +397,14 @@ exif_mnote_data_olympus_load (ExifMnoteD
 		      "Loading entry 0x%x ('%s')...", n->entries[i].tag,
 		      mnote_olympus_tag_get_name (n->entries[i].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[i].format) &&
+		buf_size / exif_format_get_size (n->entries[i].format) < n->entries[i].components
+	    ) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteOlympus", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[i].format), n->entries[i].components);
+		continue;
+	    }
 	    /*
 	     * Size? If bigger than 4 bytes, the actual data is not
 	     * in the entry but somewhere else (offset).
@@ -386,7 +414,12 @@ exif_mnote_data_olympus_load (ExifMnoteD
 	    if (!s) continue;
 	    o += 8;
 	    if (s > 4) o = exif_get_long (buf + o, n->order) + datao;
-	    if (o + s > buf_size) continue;
+            if (CHECKOVERFLOW(o, buf_size, s)) {
+		exif_log (en->log, EXIF_LOG_CODE_DEBUG,
+			"ExifMnoteOlympus", "Tag data past end of buffer (%u > %u)",
+			(unsigned)(o + s), buf_size);
+		continue;
+	    }
 
 	    /* Sanity check */
 	    n->entries[i].data = exif_mem_alloc (en->mem, s);
Index: libexif-0.6.17/libexif/pentax/exif-mnote-data-pentax.c
===================================================================
--- libexif-0.6.17.orig/libexif/pentax/exif-mnote-data-pentax.c
+++ libexif-0.6.17/libexif/pentax/exif-mnote-data-pentax.c
@@ -30,6 +30,8 @@
 
 /* #define DEBUG */
 
+#define CHECKOVERFLOW(offset,datasize,structsize) (( offset >= datasize) || (structsize > datasize) || (offset > datasize - structsize ))
+
 static void
 exif_mnote_data_pentax_clear (ExifMnoteDataPentax *n)
 {
@@ -73,9 +75,20 @@ exif_mnote_data_pentax_load (ExifMnoteDa
 		const unsigned char *buf, unsigned int buf_size)
 {
 	ExifMnoteDataPentax *n = (ExifMnoteDataPentax *) en;
-	size_t i, o, s, datao = 6 + n->offset, base = 0;
+	size_t i, o, s, datao, base = 0;
 	ExifShort c;
 
+        if (!n || !buf || !buf_size) {
+                exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteDataPentax", "Short MakerNote");
+		return;
+	}
+	datao = 6 + n->offset;
+	if (CHECKOVERFLOW(datao, buf_size, 8)) {
+		exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+		"ExifMnoteDataPentax", "Short MakerNote");
+		return;
+	}
+
 	/* Number of entries */
 	if (buf_size < datao + (4 + 2) + 2) return;
 	if (!memcmp(buf + datao, "AOC", 4)) {
@@ -106,12 +119,26 @@ exif_mnote_data_pentax_load (ExifMnoteDa
 		o = datao + 2 + 12 * i;
 		if (o + 8 > buf_size) return;
 
+		if (CHECKOVERFLOW(o,buf_size,12)) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifMnoteDataPentax", "Short MakerNote");
+			break;
+		}
+
 		n->count = i + 1;
 		n->entries[i].tag        = exif_get_short (buf + o + 0, n->order) + base;
 		n->entries[i].format     = exif_get_short (buf + o + 2, n->order);
 		n->entries[i].components = exif_get_long  (buf + o + 4, n->order);
 		n->entries[i].order      = n->order;
 
+		/* 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[i].format) &&
+			buf_size / exif_format_get_size (n->entries[i].format) < n->entries[i].components
+		) {
+			exif_log (en->log, EXIF_LOG_CODE_CORRUPT_DATA,
+				  "ExifMnoteDataPentax", "Tag size overflow detected (%u * %lu)", exif_format_get_size (n->entries[i].format), n->entries[i].components);
+			break;
+		}
 		/*
 		 * Size? If bigger than 4 bytes, the actual data is not
 		 * in the entry but somewhere else (offset).
@@ -127,8 +154,11 @@ exif_mnote_data_pentax_load (ExifMnoteDa
 		if (!s) return;
 		o += 8;
 		if (s > 4) o = exif_get_long (buf + o, n->order) + 6;
-		if (o + s > buf_size) return;
-                                                                                
+		if (CHECKOVERFLOW(o, buf_size, s)) {
+			exif_log (en->log, EXIF_LOG_CODE_DEBUG, "ExifMnoteDataPentax", "Tag data past end of buffer (%u > %u)", (unsigned)(o + s), buf_size);
+			continue;
+		}
+
 		/* Sanity check */
 		n->entries[i].data = exif_mem_alloc (en->mem, s);
 		if (!n->entries[i].data) return;
