Merge pull request #946 from kevinbackhouse/mrmimage_bounds_checking

mrwimage bounds checking

Author: piponazo

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_,

test/data/issue_943_poc1.mrm

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, path
+
+
+class MrmImageLargeAllocation(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/pull/943
+    """
+    url = "https://github.com/Exiv2/exiv2/pull/943"
+
+    filename1 = path("$data_path/issue_943_poc1.mrm")
+    filename2 = path("$data_path/issue_943_poc2.mrm")
+    commands = ["$exiv2 $filename1", "$exiv2 $filename2"]
+    stdout = ["",""]
+    stderr = [
+"""Exiv2 exception in print action for file $filename1:
+Failed to read image data
+""",
+"""Exiv2 exception in print action for file $filename2:
+Failed to read image data
+"""
+]
+    retval = [1,1]