Merge pull request #1758 from kevinbackhouse/Fix-GHSA-h9x9-4f77-336w

Check that the iterator is not end() after calling findKey
Exiv2 · Jul 5, 2021 · 75a1832 · 75a1832
2 parents 8120ff7 + 3ac426b
commit 75a1832
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 36 deletions.
diff --git a/src/convert.cpp b/src/convert.cpp
@@ -665,16 +665,17 @@ namespace Exiv2 {
 
         if (subsecTag) {
             ExifData::iterator subsec_pos = exifData_->findKey(ExifKey(subsecTag));
-            if (   subsec_pos != exifData_->end()
-                && subsec_pos->typeId() == asciiString) {
-                std::string ss = subsec_pos->toString();
-                if (!ss.empty()) {
-                    bool ok = false;
-                    stringTo<long>(ss, ok);
-                    if (ok) subsec = std::string(".") + ss;
+            if (subsec_pos != exifData_->end()) {
+                if (subsec_pos->typeId() == asciiString) {
+                    std::string ss = subsec_pos->toString();
+                    if (!ss.empty()) {
+                        bool ok = false;
+                        stringTo<long>(ss, ok);
+                        if (ok) subsec = std::string(".") + ss;
+                    }
                 }
+                if (erase_) exifData_->erase(subsec_pos);
             }
-            if (erase_) exifData_->erase(subsec_pos);
         }
 
         if (subsec.size() > 10) subsec = subsec.substr(0, 10);
@@ -1027,18 +1028,20 @@ namespace Exiv2 {
 #endif
         }
         pos = xmpData_->findKey(XmpKey(std::string(from) + "/exif:RedEyeMode"));
-        if (pos != xmpData_->end() && pos->count() > 0) {
-            int red = pos->toLong();
-            if (pos->value().ok())
-                value |= (red & 1) << 6;
+        if (pos != xmpData_->end()) {
+            if (pos->count() > 0) {
+                int red = pos->toLong();
+                if (pos->value().ok())
+                    value |= (red & 1) << 6;
 #ifndef SUPPRESS_WARNINGS
-            else
-                EXV_WARNING << "Failed to convert " << std::string(from) + "/exif:RedEyeMode" << " to " << to << "\n";
+                else
+                    EXV_WARNING << "Failed to convert " << std::string(from) + "/exif:RedEyeMode" << " to " << to << "\n";
 #endif
+            }
+            if (erase_) xmpData_->erase(pos);
         }
 
         (*exifData_)[to] = value;
-        if (erase_) xmpData_->erase(pos);
     }
 
     void Converter::cnvXmpGPSCoord(const char* from, const char* to)

diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp
@@ -1100,8 +1100,16 @@ namespace Exiv2 {
         if (ed2 != edEnd) size += ed2->size();
         if (size != 0) {
             DataBuf buf(size);
-            if (ed1 != edEnd) ed1->copy(buf.pData_, pHead->byteOrder());
-            if (ed2 != edEnd) ed2->copy(buf.pData_ + ed1->size(), pHead->byteOrder());
+            long pos = 0;
+            if (ed1 != edEnd) {
+                ed1->copy(buf.pData_, pHead->byteOrder());
+                pos += ed1->size();
+            }
+            if (ed2 != edEnd) {
+                ed2->copy(buf.pData_ + pos, pHead->byteOrder());
+                pos += ed2->size();
+            }
+            assert(pos == size);
             pHead->add(pCrwMapping->crwTagId_, pCrwMapping->crwDir_, buf);
         }
         else {

diff --git a/src/exif.cpp b/src/exif.cpp
@@ -579,8 +579,8 @@ namespace Exiv2 {
         ExifKey exifKey(key);
         iterator pos = findKey(exifKey);
         if (pos == end()) {
-            add(Exifdatum(exifKey));
-            pos = findKey(exifKey);
+            exifMetadata_.push_back(Exifdatum(exifKey));
+            return exifMetadata_.back();
         }
         return *pos;
     }

diff --git a/src/iptc.cpp b/src/iptc.cpp
@@ -263,8 +263,8 @@ namespace Exiv2 {
         IptcKey iptcKey(key);
         iterator pos = findKey(iptcKey);
         if (pos == end()) {
-            add(Iptcdatum(iptcKey));
-            pos = findKey(iptcKey);
+            iptcMetadata_.push_back(Iptcdatum(iptcKey));
+            return iptcMetadata_.back();
         }
         return *pos;
     }

diff --git a/src/pentaxmn_int.cpp b/src/pentaxmn_int.cpp
@@ -1213,6 +1213,25 @@ namespace Exiv2 {
         return result;
     }
 
+    // Exception thrown by findLensInfo when the lens info can't be found.
+    class LensInfoNotFound : public std::exception {
+    public:
+      LensInfoNotFound() {}
+    };
+
+    // Throws std::exception if the LensInfo can't be found.
+    static ExifData::const_iterator findLensInfo(const ExifData* metadata) {
+      const ExifData::const_iterator dngLensInfo = metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo"));
+      if (dngLensInfo != metadata->end()) {
+        return dngLensInfo;
+      }
+      const ExifData::const_iterator lensInfo = metadata->findKey(ExifKey("Exif.Pentax.LensInfo"));
+      if (lensInfo != metadata->end()) {
+        return lensInfo;
+      }
+      throw LensInfoNotFound();
+    }
+
     //! resolveLens0x32c print lens in human format
     std::ostream& resolveLens0x32c(std::ostream& os, const Value& value,
                                                  const ExifData* metadata)
@@ -1249,11 +1268,7 @@ namespace Exiv2 {
             unsigned long index  = 0;
 
             // http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/Pentax.html#LensData
-            const ExifData::const_iterator lensInfo = metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo")) != metadata->end()
-                                                    ? metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo"))
-                                                    : metadata->findKey(ExifKey("Exif.Pentax.LensInfo"))
-                                                    ;
-            if ( lensInfo == metadata->end() ) return EXV_PRINT_COMBITAG_MULTI(pentaxLensType, 2, 1, 2)(os, value, metadata);
+            const ExifData::const_iterator lensInfo = findLensInfo(metadata);
             if ( lensInfo->count() < 5       ) return EXV_PRINT_COMBITAG_MULTI(pentaxLensType, 2, 1, 2)(os, value, metadata);
 
             if ( value.count() == 2 ) {
@@ -1307,10 +1322,7 @@ namespace Exiv2 {
         try {
             unsigned long index  = 0;
 
-            const ExifData::const_iterator lensInfo = metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo")) != metadata->end()
-                                                    ? metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo"))
-                                                    : metadata->findKey(ExifKey("Exif.Pentax.LensInfo"))
-                                                    ;
+            const ExifData::const_iterator lensInfo = findLensInfo(metadata);
             if ( value.count() == 4 ) {
                 std::string model       = getKeyString("Exif.Image.Model"      ,metadata);
                 if ( model.rfind("PENTAX K-3", 0)==0 && lensInfo->count() == 128 && lensInfo->toLong(1) == 168 && lensInfo->toLong(2) == 144 ) index = 7;
@@ -1335,10 +1347,7 @@ namespace Exiv2 {
         try {
             unsigned long index  = 0;
 
-            const ExifData::const_iterator lensInfo = metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo")) != metadata->end()
-                                                    ? metadata->findKey(ExifKey("Exif.PentaxDng.LensInfo"))
-                                                    : metadata->findKey(ExifKey("Exif.Pentax.LensInfo"))
-                                                    ;
+            const ExifData::const_iterator lensInfo = findLensInfo(metadata);
             if ( value.count() == 4 ) {
                 std::string model       = getKeyString("Exif.Image.Model"      ,metadata);
                 if ( model.rfind("PENTAX K-3", 0)==0 && lensInfo->count() == 128 && lensInfo->toLong(1) == 131 && lensInfo->toLong(2) == 128 )

diff --git a/src/xmp.cpp b/src/xmp.cpp
@@ -308,8 +308,8 @@ namespace Exiv2 {
         XmpKey xmpKey(key);
         iterator pos = findKey(xmpKey);
         if (pos == end()) {
-            add(Xmpdatum(xmpKey));
-            pos = findKey(xmpKey);
+            xmpMetadata_.push_back(Xmpdatum(xmpKey));
+            return xmpMetadata_.back();
         }
         return *pos;
     }

diff --git a/test/data/issue_ghsa_h9x9_4f77_336w_poc.exv b/test/data/issue_ghsa_h9x9_4f77_336w_poc.exv
diff --git a/tests/bugfixes/github/test_issue_ghsa_h9x9_4f77_336w.py b/tests/bugfixes/github/test_issue_ghsa_h9x9_4f77_336w.py
@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, CopyTmpFiles, path, check_no_ASAN_UBSAN_errors
+
+class Jp2ImageEncodeJp2HeaderOutOfBoundsRead2(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/security/advisories/GHSA-h9x9-4f77-336w
+    """
+    url = "https://github.com/Exiv2/exiv2/security/advisories/GHSA-h9x9-4f77-336w"
+
+    filename = path("$data_path/issue_ghsa_h9x9_4f77_336w_poc.exv")
+    commands = ["$exiv2 -P t $filename"]
+    retval = [0]
+
+    compare_stdout = check_no_ASAN_UBSAN_errors
+    compare_stderr = check_no_ASAN_UBSAN_errors