Merge pull request #2346 from kevinbackhouse/fix-issue-2345

Fix buffer overflow in QuickTimeVideo::tagDecoder

src/quicktimevideo.cpp

@@ -568,10 +568,16 @@ void QuickTimeVideo::decodeBlock(std::string const& entered_from) {
   enforce(size - hdrsize <= std::numeric_limits<size_t>::max(), Exiv2::ErrorCode::kerCorruptedMetadata);
 
   // std::cerr<<"Tag=>"<<buf.data()<<"     size=>"<<size-hdrsize << std::endl;
-  tagDecoder(buf, static_cast<size_t>(size - hdrsize));
+  const size_t newsize = static_cast<size_t>(size - hdrsize);
+  if (newsize > buf.size()) {
+    buf.resize(newsize);
+  }
+  tagDecoder(buf, newsize);
 }  // QuickTimeVideo::decodeBlock
 
 void QuickTimeVideo::tagDecoder(Exiv2::DataBuf& buf, size_t size) {
+  assert(buf.size() > 4);
+
   if (ignoreList(buf))
     discard(size);
 
@@ -620,24 +626,30 @@ void QuickTimeVideo::tagDecoder(Exiv2::DataBuf& buf, size_t size) {
     keysTagDecoder(size);
 
   else if (equalsQTimeTag(buf, "url ")) {
-    io_->readOrThrow(buf.data(), size);
+    Exiv2::DataBuf url(size + 1);
+    io_->readOrThrow(url.data(), size);
+    url.write_uint8(size, 0);
     if (currentStream_ == Video)
-      xmpData_["Xmp.video.URL"] = Exiv2::toString(buf.data());
+      xmpData_["Xmp.video.URL"] = Exiv2::toString(url.data());
     else if (currentStream_ == Audio)
-      xmpData_["Xmp.audio.URL"] = Exiv2::toString(buf.data());
+      xmpData_["Xmp.audio.URL"] = Exiv2::toString(url.data());
   }
 
   else if (equalsQTimeTag(buf, "urn ")) {
-    io_->readOrThrow(buf.data(), size);
+    Exiv2::DataBuf urn(size + 1);
+    io_->readOrThrow(urn.data(), size);
+    urn.write_uint8(size, 0);
     if (currentStream_ == Video)
-      xmpData_["Xmp.video.URN"] = Exiv2::toString(buf.data());
+      xmpData_["Xmp.video.URN"] = Exiv2::toString(urn.data());
     else if (currentStream_ == Audio)
-      xmpData_["Xmp.audio.URN"] = Exiv2::toString(buf.data());
+      xmpData_["Xmp.audio.URN"] = Exiv2::toString(urn.data());
   }
 
   else if (equalsQTimeTag(buf, "dcom")) {
-    io_->readOrThrow(buf.data(), size);
+    Exiv2::DataBuf dcom(size + 1);
-    xmpData_["Xmp.video.Compressor"] = Exiv2::toString(buf.data());
+    io_->readOrThrow(dcom.data(), size);
+    dcom.write_uint8(size, 0);
+    xmpData_["Xmp.video.Compressor"] = Exiv2::toString(dcom.data());
   }
 
   else if (equalsQTimeTag(buf, "smhd")) {

tests/bugfixes/github/test_issue_2345.py

@@ -0,0 +1,13 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, check_no_ASAN_UBSAN_errors
+
+class issue_2345_QuickTimeVideo_tagDecoder_buffer_overflow(metaclass=CaseMeta):
+    url      = "https://github.com/Exiv2/exiv2/issues/2345"
+    filename = "$data_path/issue_2345_poc.mp4"
+    commands = ["$exiv2 -q -pa $filename"]
+    retval   = [1]
+    stderr   = ["""$exiv2_exception_message $filename:
+$kerCorruptedMetadata
+"""]
+    compare_stdout = check_no_ASAN_UBSAN_errors

tests/regression_tests/test_regression_allfiles.py

@@ -59,6 +59,7 @@ def get_valid_files(data_dir):
         # non-zero return code files, most of them are security POC so we don't
         # really need to worry about them here
         "issue_2340_poc.mp4",
+        "issue_2345_poc.mp4",
         "2018-01-09-exiv2-crash-001.tiff",
         "cve_2017_1000126_stack-oob-read.webp",
         "exiv2-bug1247.jpg",