From cb7dc5a5280958920348656be67f6910caf21d4f Mon Sep 17 00:00:00 2001
From: Kevin Backhouse <kev@semmle.com>
Date: Fri, 5 Jul 2019 15:41:14 +0100
Subject: [PATCH] Check for integer overflows in mrwimage.cpp

---
 src/mrwimage.cpp                        |  22 +++++++++++++++-------
 test/data/issue_943_poc.mrm             | Bin 0 -> 37 bytes
 tests/bugfixes/github/test_issue_943.py |  21 +++++++++++++++++++++
 3 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 test/data/issue_943_poc.mrm
 create mode 100644 tests/bugfixes/github/test_issue_943.py

diff --git a/src/mrwimage.cpp b/src/mrwimage.cpp
index 29ec4092..5a3d84af 100644
--- a/src/mrwimage.cpp
+++ b/src/mrwimage.cpp
@@ -32,6 +32,7 @@
 #include "image.hpp"
 #include "basicio.hpp"
 #include "error.hpp"
+#include "enforce.hpp"
 #include "futils.hpp"
 
 // + standard includes
@@ -114,26 +115,33 @@ namespace Exiv2 {
         uint32_t const end = getULong(tmp + 4, bigEndian);
 
         pos += len;
-        if (pos > end) throw Error(kerFailedToReadImageData);
+        enforce(pos <= end, kerFailedToReadImageData);
         io_->read(tmp, len);
         if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData);
 
         while (memcmp(tmp + 1, "TTW", 3) != 0) {
             uint32_t const siz = getULong(tmp + 4, bigEndian);
+            enforce(siz <= end - pos, kerFailedToReadImageData);
             pos += siz;
-            if (pos > end) throw Error(kerFailedToReadImageData);
             io_->seek(siz, BasicIo::cur);
-            if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData);
+            enforce(!io_->error() && !io_->eof(), kerFailedToReadImageData);
 
+            enforce(len <= end - pos, kerFailedToReadImageData);
             pos += len;
-            if (pos > end) throw Error(kerFailedToReadImageData);
             io_->read(tmp, len);
-            if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData);
+            enforce(!io_->error() && !io_->eof(), kerFailedToReadImageData);
         }
 
-        DataBuf buf(getULong(tmp + 4, bigEndian));
+        const uint32_t siz = getULong(tmp + 4, bigEndian);
+        // First do an approximate bounds check of siz, so that we don't
+        // get DOS-ed by a 4GB allocation on the next line. If siz is
+        // greater than io_->size() then it is definitely invalid. But the
+        // exact bounds checking is done by the call to io_->read, which
+        // will fail if there are fewer than siz bytes left to read.
+        enforce(siz <= io_->size(), kerFailedToReadImageData);
+        DataBuf buf(siz);
         io_->read(buf.pData_, buf.size_);
-        if (io_->error() || io_->eof()) throw Error(kerFailedToReadImageData);
+        enforce(!io_->error() && !io_->eof(), kerFailedToReadImageData);
 
         ByteOrder bo = TiffParser::decode(exifData_,
                                           iptcData_,
diff --git a/test/data/issue_943_poc.mrm b/test/data/issue_943_poc.mrm
new file mode 100644
index 0000000000000000000000000000000000000000..a5a44587e49e8d71400595ae301ad202a47e5f2f
GIT binary patch
literal 37
ZcmZSZ4f16Nfsl~!{}8|jrC}6<5&&<77%2b%

literal 0
HcmV?d00001

diff --git a/tests/bugfixes/github/test_issue_943.py b/tests/bugfixes/github/test_issue_943.py
new file mode 100644
index 00000000..7ee2f18d
--- /dev/null
+++ b/tests/bugfixes/github/test_issue_943.py
@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, path
+
+
+class OutOfBoundsReadInIptcParserDecode(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/pull/943
+    """
+    url = "https://github.com/Exiv2/exiv2/pull/943"
+
+    filename = path("$data_path/issue_943_poc.mrm")
+    commands = ["$exiv2 $filename"]
+    stdout = [""]
+    stderr = [
+        """Exiv2 exception in print action for file $filename:
+Failed to read image data
+"""
+]
+    retval = [1]