Merge pull request #1889 from Exiv2/mergify/bp/0.27-maintenance/pr-1888

Avoid reading 1 byte off the end when the string does not contain a '\0' byte (backport #1888)
Exiv2 · Sep 5, 2021 · 4921385 · 4921385
2 parents b9e35f6 + a95fe1f
commit 4921385
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 9 deletions.
diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp
@@ -847,22 +847,20 @@ namespace Exiv2 {
         ExifKey key1("Exif.Image.Make");
         Value::AutoPtr value1 = Value::create(ciffComponent.typeId());
         uint32_t i = 0;
-        for (;    i < ciffComponent.size()
-               && ciffComponent.pData()[i] != '\0'; ++i) {
+        while (i < ciffComponent.size() && ciffComponent.pData()[i++] != '\0') {
             // empty
         }
-        value1->read(ciffComponent.pData(), ++i, byteOrder);
+        value1->read(ciffComponent.pData(), i, byteOrder);
         image.exifData().add(key1, value1.get());
 
         // Model
         ExifKey key2("Exif.Image.Model");
         Value::AutoPtr value2 = Value::create(ciffComponent.typeId());
         uint32_t j = i;
-        for (;    i < ciffComponent.size()
-               && ciffComponent.pData()[i] != '\0'; ++i) {
+        while (i < ciffComponent.size() && ciffComponent.pData()[i++] != '\0') {
             // empty
         }
-        value2->read(ciffComponent.pData() + j, i - j + 1, byteOrder);
+        value2->read(ciffComponent.pData() + j, i - j, byteOrder);
         image.exifData().add(key2, value2.get());
     } // CrwMap::decode0x080a
 
@@ -1000,11 +998,10 @@ namespace Exiv2 {
             else if (ciffComponent.typeId() == asciiString) {
                 // determine size from the data, by looking for the first 0
                 uint32_t i = 0;
-                for (;    i < ciffComponent.size()
-                       && ciffComponent.pData()[i] != '\0'; ++i) {
+                while (i < ciffComponent.size() && ciffComponent.pData()[i++] != '\0') {
                     // empty
                 }
-                size = ++i;
+                size = i;
             }
             else {
                 // by default, use the size from the directory entry

diff --git a/test/data/issue_1887_poc.crw b/test/data/issue_1887_poc.crw
diff --git a/tests/bugfixes/github/test_issue_1887.py b/tests/bugfixes/github/test_issue_1887.py
@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, path, check_no_ASAN_UBSAN_errors
+
+
+class OutOfBoundsReadInCrwMapDecode0x080a(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/issues/1887
+    """
+    url = "https://github.com/Exiv2/exiv2/issues/1887"
+
+    filename = path("$data_path/issue_1887_poc.crw")
+    commands = ["$exiv2 $filename"]
+    stderr = [""]
+    retval = [0]
+
+    compare_stdout = check_no_ASAN_UBSAN_errors