From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Tue, 24 Aug 2021 18:04:56 +0000 Subject: [PATCH] Fix MakerNote tag size overflow issues at read time. This is a cherry-pick of https://github.com/libexif/libexif/commit/435e21f05001fb03f9f186fa7cbc69454afd00d1 for CVE-2020-13112 Bug: 194342672 Test: sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.CVE_2020_13112#testPocBug_194342672 Change-Id: Ibdf388bc768213833f8fef9740b3527d46a14a2a Merged-In: Id106e79e829329145d27a93273241b58878bfac3 Signed-off-by: Jayant Chowdhary (cherry picked from commit fd5f7bab830858e57a2baf9d4dd47e5820337b56) Merged-In:Ibdf388bc768213833f8fef9740b3527d46a14a2a --- libexif/canon/exif-mnote-data-canon.c | 20 ++++++++++++++--- libexif/fuji/exif-mnote-data-fuji.c | 22 +++++++++++++----- libexif/olympus/exif-mnote-data-olympus.c | 27 ++++++++++++++++++----- libexif/pentax/exif-mnote-data-pentax.c | 19 ++++++++++++---- 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 #include +#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);