Relevant commit for CVE-2019-9278

[utkarsh2102]

As the CVE quotes,

In libexif, there is a possible out of bounds write due to an integer overflow. This could lead to remote escalation of privilege in the media content provider with no additional execution privileges needed. User interaction is needed for exploitation.

Do we have a fix for it yet?
Relevant bug report at Debian Security Tracker: https://security-tracker.debian.org/tracker/CVE-2019-9278

[utkarsh2102]

Patch here:

--- a/libexif/exif-data.c
+++ b/libexif/exif-data.c

@@ -35,6 +35,7 @@
 #include <libexif/olympus/exif-mnote-data-olympus.h>
 #include <libexif/pentax/exif-mnote-data-pentax.h>
 
+#include <inttypes.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -191,9 +192,12 @@
 		doff = offset + 8;
 
 	/* Sanity checks */
-	if ((doff + s < doff) || (doff + s < s) || (doff + s > size)) {
+	int64_t doff64 = doff;
+	int64_t s64 = s;
+	if (doff64 + s64 > (int64_t) size) {
 		exif_log (data->priv->log, EXIF_LOG_CODE_DEBUG, "ExifData",
-				  "Tag data past end of buffer (%u > %u)", doff+s, size);	
+				  "Tag data past end of buffer (%" PRId64 " > %u)",
+				  doff64+s64, size);
 		return 0;
 	}
 
@@ -904,7 +908,7 @@
 		  "IFD 0 at %i.", (int) offset);
 
 	/* Sanity check the offset, being careful about overflow */
-	if (offset > ds || offset + 6 + 2 > ds)
+	if (offset > ds || (uint64_t)offset + 6 + 2 > ds)
 		return;
 
 	/* Parse the actual exif data (usually offset 14 from start) */
@@ -912,7 +916,7 @@
 
 	/* IFD 1 offset */
 	n = exif_get_short (d + 6 + offset, data->priv->order);
-	if (offset + 6 + 2 + 12 * n + 4 > ds)
+	if ((uint64_t)offset + 6 + 2 + 12 * n + 4 > ds)
 		return;
 
 	offset = exif_get_long (d + 6 + offset + 2 + 12 * n, data->priv->order);
@@ -921,7 +925,7 @@
 			  "IFD 1 at %i.", (int) offset);
 
 		/* Sanity check. */
-		if (offset > ds || offset + 6 > ds) {
+		if (offset > ds || (uint64_t)offset + 6 > ds) {
 			exif_log (data->priv->log, EXIF_LOG_CODE_CORRUPT_DATA,
 				  "ExifData", "Bogus offset of IFD1.");
 		} else {

Source: https://android.googlesource.com/platform/external/libexif/+/a5e8e5812a11ec9686294de8a5d68aaf2ab72475%5E%21/#F0

[dfandrich]

This is the first I've heard of this CVE.

[utkarsh2102]

Hi @dfandrich,
Could you see and confirm if this patch is the actual fix for this CVE?

[dfandrich]

This looks like the Android patch, yet. I haven't analyzed whether it fixes the alleged problem or not.

[utkarsh2102]

Ah, well. Hope you fix it at the earliest. I need to fix this in Debian :)

[utkarsh2102]

@dfandrich,
Could you give me an update?

[dfandrich]

I haven't had time to look at it yet.

[utkarsh2102]

Hi @dfandrich,
Sorry for pinging you again, but did you have time to take a look at this yet?

[utkarsh2102]

Hi @dfandrich,
Could you please take a look into this now?
Been over a month..

[msmeissn]

do you have the testcase available?
I would like a bit smaller approach

[msmeissn]

not sure if
(doff > size) || (size - doff < s)

would be safe already as it avoids the overflowing addition

[msmeissn]

well,
https://stackoverflow.com/questions/16056758/c-c-unsigned-integer-overflow

so it should work wjhen operting on unsigned ints...
... but in general checking for overflow after the fact is definitely a recipe for failure , so avoid doing that.