diff --git a/include/exiv2/bmffimage.hpp b/include/exiv2/bmffimage.hpp index b634953925..786f57201b 100644 --- a/include/exiv2/bmffimage.hpp +++ b/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 ilocs_; bool bReadMetadata_{false}; + const size_t max_box_depth_; //@} /*! diff --git a/include/exiv2/quicktimevideo.hpp b/include/exiv2/quicktimevideo.hpp index 3d3d23bd0f..e40e448ca1 100644 --- a/include/exiv2/quicktimevideo.hpp +++ b/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 diff --git a/src/bmffimage.cpp b/src/bmffimage.cpp index 063861896a..ac4ddd30db 100644 --- a/src/bmffimage.cpp +++ b/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); diff --git a/src/quicktimevideo.cpp b/src/quicktimevideo.cpp index fccc08dc9f..d87d1e87a4 100644 --- a/src/quicktimevideo.cpp +++ b/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(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 diff --git a/test/data/issue_ghsa_crmj_qh74_2r36_poc.mov b/test/data/issue_ghsa_crmj_qh74_2r36_poc.mov new file mode 100644 index 0000000000..245cc14bce Binary files /dev/null and b/test/data/issue_ghsa_crmj_qh74_2r36_poc.mov differ diff --git a/tests/bugfixes/github/test_issue_ghsa_crmj_qh74_2r36.py b/tests/bugfixes/github/test_issue_ghsa_crmj_qh74_2r36.py new file mode 100644 index 0000000000..704e624e93 --- /dev/null +++ b/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 = [""] diff --git a/tests/regression_tests/test_regression_allfiles.py b/tests/regression_tests/test_regression_allfiles.py index 49ece7d635..eb7f7cef2d 100644 --- a/tests/regression_tests/test_regression_allfiles.py +++ b/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",