Merge pull request #2915 from kevinbackhouse/fix-GHSA-crmj-qh74-2r36

prevent unbounded recursion in QuickTimeVideo::multipleEntriesDecoder

include/exiv2/bmffimage.hpp

@@ -56,7 +56,7 @@ class EXIV2API BmffImage : public Image {
     @param create Specifies if an existing image should be read (false)
         or if a new file should be created (true).
    */
-  BmffImage(BasicIo::UniquePtr io, bool create);
+  BmffImage(BasicIo::UniquePtr io, bool create, size_t max_box_depth = 1000);
   //@}
 
   //@{
@@ -138,6 +138,7 @@ class EXIV2API BmffImage : public Image {
   uint16_t xmpID_{0};
   std::map<uint32_t, Iloc> ilocs_;
   bool bReadMetadata_{false};
+  const size_t max_box_depth_;
   //@}
 
   /*!

include/exiv2/quicktimevideo.hpp

@@ -52,7 +52,7 @@ class EXIV2API QuickTimeVideo : public Image {
         instance after it is passed to this method. Use the Image::io()
         method to get a temporary reference.
    */
-  explicit QuickTimeVideo(BasicIo::UniquePtr io);
+  explicit QuickTimeVideo(BasicIo::UniquePtr io, size_t max_recursion_depth = 1000);
   //@}
 
   //! @name Manipulators
@@ -71,7 +71,7 @@ class EXIV2API QuickTimeVideo : public Image {
     @brief Check for a valid tag and decode the block at the current IO
     position. Calls tagDecoder() or skips to next tag, if required.
    */
-  void decodeBlock(std::string const& entered_from = "");
+  void decodeBlock(size_t recursion_depth, std::string const& entered_from = "");
   /*!
     @brief Interpret tag information, and call the respective function
         to save it in the respective XMP container. Decodes a Tag
@@ -80,7 +80,7 @@ class EXIV2API QuickTimeVideo : public Image {
     @param buf Data buffer which contains tag ID.
     @param size Size of the data block used to store Tag Information.
    */
-  void tagDecoder(Exiv2::DataBuf& buf, size_t size);
+  void tagDecoder(Exiv2::DataBuf& buf, size_t size, size_t recursion_depth);
 
  private:
   /*!
@@ -123,7 +123,7 @@ class EXIV2API QuickTimeVideo : public Image {
     @brief Interpret Tag which contain other sub-tags,
         and save it in the respective XMP container.
    */
-  void multipleEntriesDecoder();
+  void multipleEntriesDecoder(size_t recursion_depth);
   /*!
     @brief Interpret Sample Description Tag, and save it
         in the respective XMP container.
@@ -140,7 +140,7 @@ class EXIV2API QuickTimeVideo : public Image {
         in the respective XMP container.
     @param size Size of the data block used to store Tag Information.
    */
-  void userDataDecoder(size_t size);
+  void userDataDecoder(size_t size, size_t recursion_depth);
   /*!
     @brief Interpret Preview Tag, and save it
         in the respective XMP container.
@@ -202,6 +202,8 @@ class EXIV2API QuickTimeVideo : public Image {
   //! Variable to store height and width of a video frame.
   uint64_t height_ = 0;
   uint64_t width_ = 0;
+  //! Prevent stack exhaustion due to excessively deep recursion.
+  const size_t max_recursion_depth_;
 
 };  // QuickTimeVideo End
 

src/bmffimage.cpp

@@ -86,8 +86,8 @@ std::string Iloc::toString() const {
   return Internal::stringFormat("ID = %u from,length = %u,%u", ID_, start_, length_);
 }
 
-BmffImage::BmffImage(BasicIo::UniquePtr io, bool /* create */) :
-    Image(ImageType::bmff, mdExif | mdIptc | mdXmp, std::move(io)) {
+BmffImage::BmffImage(BasicIo::UniquePtr io, bool /* create */, size_t max_box_depth) :
+    Image(ImageType::bmff, mdExif | mdIptc | mdXmp, std::move(io)), max_box_depth_(max_box_depth) {
 }  // BmffImage::BmffImage
 
 std::string BmffImage::toAscii(uint32_t n) {
@@ -247,7 +247,7 @@ uint64_t BmffImage::boxHandler(std::ostream& out /* = std::cout*/, Exiv2::PrintS
   // never visit a box twice!
   if (depth == 0)
     visits_.clear();
-  if (visits_.find(address) != visits_.end() || visits_.size() > visits_max_) {
+  if (visits_.find(address) != visits_.end() || visits_.size() > visits_max_ || depth >= max_box_depth_) {
     throw Error(ErrorCode::kerCorruptedMetadata);
   }
   visits_.insert(address);

src/quicktimevideo.cpp

@@ -538,8 +538,11 @@ namespace Exiv2 {
 
 using namespace Exiv2::Internal;
 
-QuickTimeVideo::QuickTimeVideo(BasicIo::UniquePtr io) :
-    Image(ImageType::qtime, mdNone, std::move(io)), timeScale_(1), currentStream_(Null) {
+QuickTimeVideo::QuickTimeVideo(BasicIo::UniquePtr io, size_t max_recursion_depth) :
+    Image(ImageType::qtime, mdNone, std::move(io)),
+    timeScale_(1),
+    currentStream_(Null),
+    max_recursion_depth_(max_recursion_depth) {
 }  // QuickTimeVideo::QuickTimeVideo
 
 std::string QuickTimeVideo::mimeType() const {
@@ -569,12 +572,14 @@ void QuickTimeVideo::readMetadata() {
   xmpData_["Xmp.video.MimeType"] = mimeType();
 
   while (continueTraversing_)
-    decodeBlock();
+    decodeBlock(0);
 
   xmpData_["Xmp.video.AspectRatio"] = getAspectRatio(width_, height_);
 }  // QuickTimeVideo::readMetadata
 
-void QuickTimeVideo::decodeBlock(std::string const& entered_from) {
+void QuickTimeVideo::decodeBlock(size_t recursion_depth, std::string const& entered_from) {
+  enforce(recursion_depth < max_recursion_depth_, Exiv2::ErrorCode::kerCorruptedMetadata);
+
   const long bufMinSize = 4;
   DataBuf buf(bufMinSize + 1);
   uint64_t size = 0;
@@ -613,7 +618,7 @@ void QuickTimeVideo::decodeBlock(std::string const& entered_from) {
   if (newsize > buf.size()) {
     buf.resize(newsize);
   }
-  tagDecoder(buf, newsize);
+  tagDecoder(buf, newsize, recursion_depth + 1);
 }  // QuickTimeVideo::decodeBlock
 
 static std::string readString(BasicIo& io, size_t size) {
@@ -624,14 +629,15 @@ static std::string readString(BasicIo& io, size_t size) {
   return Exiv2::toString(str.data());
 }
 
-void QuickTimeVideo::tagDecoder(Exiv2::DataBuf& buf, size_t size) {
+void QuickTimeVideo::tagDecoder(Exiv2::DataBuf& buf, size_t size, size_t recursion_depth) {
+  enforce(recursion_depth < max_recursion_depth_, Exiv2::ErrorCode::kerCorruptedMetadata);
   assert(buf.size() > 4);
 
   if (ignoreList(buf))
     discard(size);
 
   else if (dataIgnoreList(buf)) {
-    decodeBlock(Exiv2::toString(buf.data()));
+    decodeBlock(recursion_depth + 1, Exiv2::toString(buf.data()));
   } else if (equalsQTimeTag(buf, "ftyp"))
     fileTypeDecoder(size);
 
@@ -654,10 +660,10 @@ void QuickTimeVideo::tagDecoder(Exiv2::DataBuf& buf, size_t size) {
     videoHeaderDecoder(size);
 
   else if (equalsQTimeTag(buf, "udta"))
-    userDataDecoder(size);
+    userDataDecoder(size, recursion_depth + 1);
 
   else if (equalsQTimeTag(buf, "dref"))
-    multipleEntriesDecoder();
+    multipleEntriesDecoder(recursion_depth + 1);
 
   else if (equalsQTimeTag(buf, "stsd"))
     sampleDesc(size);
@@ -836,7 +842,8 @@ void QuickTimeVideo::CameraTagsDecoder(size_t size_external) {
   io_->seek(cur_pos + size_external, BasicIo::beg);
 }  // QuickTimeVideo::CameraTagsDecoder
 
-void QuickTimeVideo::userDataDecoder(size_t size_external) {
+void QuickTimeVideo::userDataDecoder(size_t size_external, size_t recursion_depth) {
+  enforce(recursion_depth < max_recursion_depth_, Exiv2::ErrorCode::kerCorruptedMetadata);
   size_t cur_pos = io_->tell();
   const TagVocabulary* td;
   const TagVocabulary* tv;
@@ -866,7 +873,7 @@ void QuickTimeVideo::userDataDecoder(size_t size_external) {
       break;
 
     if (equalsQTimeTag(buf, "DcMD") || equalsQTimeTag(buf, "NCDT"))
-      userDataDecoder(size - 8);
+      userDataDecoder(size - 8, recursion_depth + 1);
 
     else if (equalsQTimeTag(buf, "NCTG"))
       NikonTagsDecoder(size - 8);
@@ -898,7 +905,7 @@ void QuickTimeVideo::userDataDecoder(size_t size_external) {
     }
 
     else if (td)
-      tagDecoder(buf, size - 8);
+      tagDecoder(buf, size - 8, recursion_depth + 1);
   }
 
   io_->seek(cur_pos + size_external, BasicIo::beg);
@@ -1274,7 +1281,8 @@ void QuickTimeVideo::imageDescDecoder() {
   xmpData_["Xmp.video.BitDepth"] = static_cast<int>(buf.read_uint8(0));
 }  // QuickTimeVideo::imageDescDecoder
 
-void QuickTimeVideo::multipleEntriesDecoder() {
+void QuickTimeVideo::multipleEntriesDecoder(size_t recursion_depth) {
+  enforce(recursion_depth < max_recursion_depth_, Exiv2::ErrorCode::kerCorruptedMetadata);
   DataBuf buf(4 + 1);
   io_->readOrThrow(buf.data(), 4);
   io_->readOrThrow(buf.data(), 4);
@@ -1283,7 +1291,7 @@ void QuickTimeVideo::multipleEntriesDecoder() {
   noOfEntries = buf.read_uint32(0, bigEndian);
 
   for (uint32_t i = 0; i < noOfEntries && continueTraversing_; i++) {
-    decodeBlock();
+    decodeBlock(recursion_depth + 1);
   }
 }  // QuickTimeVideo::multipleEntriesDecoder
 

tests/bugfixes/github/test_issue_ghsa_crmj_qh74_2r36.py

@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+
+from system_tests import CaseMeta, CopyTmpFiles, path, check_no_ASAN_UBSAN_errors
+
+class QuickTimeVideoNikonTagsDecoderOutOfBoundsRead(metaclass=CaseMeta):
+    """
+    Regression test for the bug described in:
+    https://github.com/Exiv2/exiv2/security/advisories/GHSA-crmj-qh74-2r36
+    """
+    url = "https://github.com/Exiv2/exiv2/security/advisories/GHSA-crmj-qh74-2r36"
+
+    filename = path("$data_path/issue_ghsa_crmj_qh74_2r36_poc.mov")
+    commands = ["$exiv2 $filename"]
+    retval = [1]
+    stderr   = ["""$exiv2_exception_message $filename:
+$kerCorruptedMetadata
+"""]
+    stdout = [""]

tests/regression_tests/test_regression_allfiles.py

@@ -113,6 +113,8 @@ def get_valid_files(data_dir):
         "issue_2339_poc.tiff",
         "issue_2352_poc.jpg",
         "issue_2385_poc.tiff",
+        "issue_ghsa_crmj_qh74_2r36_poc.mov",
+        "issue_ghsa_g9xm_7538_mq8w_poc.mov",
         "issue_ghsa_583f_w9pm_99r2_poc.jp2",
         "issue_ghsa_7569_phvm_vwc2_poc.jp2",
         "issue_ghsa_mxw9_qx4c_6m8v_poc.jp2",