From 7d9deba07166e184ea698ea724554ebfeded6ae8 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 20 Nov 2022 15:58:55 +0000 Subject: [PATCH 1/3] Regression test for https://github.com/Exiv2/exiv2/issues/2423 --- test/data/issue_2423_poc.mp4 | Bin 0 -> 129 bytes tests/bugfixes/github/test_issue_2423.py | 13 +++++++++++++ .../regression_tests/test_regression_allfiles.py | 1 + 3 files changed, 14 insertions(+) create mode 100644 test/data/issue_2423_poc.mp4 create mode 100644 tests/bugfixes/github/test_issue_2423.py diff --git a/test/data/issue_2423_poc.mp4 b/test/data/issue_2423_poc.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..bf8171b6a7e9fd3e57710fa3d4d221636adc5383 GIT binary patch literal 129 zcmZQzU=T?wsVvCL%+6=vNG_@@@?-!4k>Zl#l$UV8z*SkEUzDO+Y<=Ft;QxQ!09U^` oA&&olFgWMu78C(dYH@LX(f_*pAic=oQ0rla{|x{CBa5N30a58E^Z)<= literal 0 HcmV?d00001 diff --git a/tests/bugfixes/github/test_issue_2423.py b/tests/bugfixes/github/test_issue_2423.py new file mode 100644 index 00000000..0c633cf6 --- /dev/null +++ b/tests/bugfixes/github/test_issue_2423.py @@ -0,0 +1,13 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, check_no_ASAN_UBSAN_errors + +class issue_2423_QuickTimeVideo_sampleDesc_long_running(metaclass=CaseMeta): + url = "https://github.com/Exiv2/exiv2/issues/2423" + filename = "$data_path/issue_2423_poc.mp4" + 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 25ba4c0e..bf2d04fb 100644 --- a/tests/regression_tests/test_regression_allfiles.py +++ b/tests/regression_tests/test_regression_allfiles.py @@ -65,6 +65,7 @@ def get_valid_files(data_dir): "issue_2377_poc.mp4", "issue_2383_poc.mp4", "issue_2393_poc.mp4", + "issue_2423_poc.mp4", "2018-01-09-exiv2-crash-001.tiff", "cve_2017_1000126_stack-oob-read.webp", "exiv2-bug1247.jpg", From 292082df6f047d1dae6fbaa7552b163cf3b778de Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 20 Nov 2022 16:01:12 +0000 Subject: [PATCH 2/3] Add break to loop to fix issue 2423, plus some other cleanups. --- include/exiv2/quicktimevideo.hpp | 8 ++++---- src/quicktimevideo.cpp | 26 ++++++++++++++------------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/exiv2/quicktimevideo.hpp b/include/exiv2/quicktimevideo.hpp index d4f342d6..a665178a 100644 --- a/include/exiv2/quicktimevideo.hpp +++ b/include/exiv2/quicktimevideo.hpp @@ -209,13 +209,13 @@ class QuickTimeVideo : public Image { private: //! Variable which stores Time Scale unit, used to calculate time. - uint64_t timeScale_; + uint64_t timeScale_ = 0; //! Variable which stores current stream being processsed. - int currentStream_; + int currentStream_ = 0; //! Variable to check the end of metadata traversing. - bool continueTraversing_; + bool continueTraversing_ = 0; //! Variable to store height and width of a video frame. - uint64_t height_, width_; + uint64_t height_ = 0, width_ = 0; }; // QuickTimeVideo End diff --git a/src/quicktimevideo.cpp b/src/quicktimevideo.cpp index b44b3683..882f19ab 100644 --- a/src/quicktimevideo.cpp +++ b/src/quicktimevideo.cpp @@ -26,6 +26,7 @@ #include "error.hpp" #include "futils.hpp" #include "quicktimevideo.hpp" +#include "safe_op.hpp" #include "tags.hpp" #include "tags_int.hpp" // + standard includes @@ -496,7 +497,8 @@ namespace Exiv2 { using namespace Exiv2::Internal; -QuickTimeVideo::QuickTimeVideo(BasicIo::UniquePtr io) : Image(ImageType::qtime, mdNone, std::move(io)), timeScale_(1) { +QuickTimeVideo::QuickTimeVideo(BasicIo::UniquePtr io) : + Image(ImageType::qtime, mdNone, std::move(io)), timeScale_(1), currentStream_(Null) { } // QuickTimeVideo::QuickTimeVideo std::string QuickTimeVideo::mimeType() const { @@ -860,8 +862,8 @@ void QuickTimeVideo::userDataDecoder(size_t size_external) { void QuickTimeVideo::NikonTagsDecoder(size_t size_external) { size_t cur_pos = io_->tell(); DataBuf buf(200), buf2(4 + 1); - unsigned long TagID = 0; - unsigned short dataLength = 0, dataType = 2; + uint32_t TagID = 0; + uint16_t dataLength = 0, dataType = 2; const TagDetails *td, *td2; for (int i = 0; i < 100; i++) { @@ -1094,13 +1096,12 @@ void QuickTimeVideo::timeToSampleDecoder() { DataBuf buf(4 + 1); io_->readOrThrow(buf.data(), 4); io_->readOrThrow(buf.data(), 4); - size_t noOfEntries, totalframes = 0, timeOfFrames = 0; - noOfEntries = buf.read_uint32(0, bigEndian); - size_t temp; + uint64_t totalframes = 0, timeOfFrames = 0; + const uint32_t noOfEntries = buf.read_uint32(0, bigEndian); - for (unsigned long i = 1; i <= noOfEntries; i++) { + for (uint32_t i = 0; i < noOfEntries; i++) { io_->readOrThrow(buf.data(), 4); - temp = buf.read_uint32(0, bigEndian); + const uint64_t temp = buf.read_uint32(0, bigEndian); totalframes += temp; io_->readOrThrow(buf.data(), 4); timeOfFrames += temp * buf.read_uint32(0, bigEndian); @@ -1114,16 +1115,17 @@ void QuickTimeVideo::sampleDesc(size_t size) { size_t cur_pos = io_->tell(); io_->readOrThrow(buf.data(), 4); io_->readOrThrow(buf.data(), 4); - size_t noOfEntries; - noOfEntries = buf.read_uint32(0, bigEndian); + const uint32_t noOfEntries = buf.read_uint32(0, bigEndian); - for (unsigned long i = 1; i <= noOfEntries; i++) { + for (uint32_t i = 0; i < noOfEntries; i++) { if (currentStream_ == Video) imageDescDecoder(); else if (currentStream_ == Audio) audioDescDecoder(); + else + break; } - io_->seek(cur_pos + size, BasicIo::beg); + io_->seek(Safe::add(cur_pos, size), BasicIo::beg); } // QuickTimeVideo::sampleDesc void QuickTimeVideo::audioDescDecoder() { From 9d044d30b17b098e6b65829bdd9ba584f6c6ccca Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 21 Nov 2022 10:50:39 +0000 Subject: [PATCH 3/3] Use Safe::add --- src/quicktimevideo.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/quicktimevideo.cpp b/src/quicktimevideo.cpp index 882f19ab..00db1fc1 100644 --- a/src/quicktimevideo.cpp +++ b/src/quicktimevideo.cpp @@ -1102,9 +1102,9 @@ void QuickTimeVideo::timeToSampleDecoder() { for (uint32_t i = 0; i < noOfEntries; i++) { io_->readOrThrow(buf.data(), 4); const uint64_t temp = buf.read_uint32(0, bigEndian); - totalframes += temp; + totalframes = Safe::add(totalframes, temp); io_->readOrThrow(buf.data(), 4); - timeOfFrames += temp * buf.read_uint32(0, bigEndian); + timeOfFrames = Safe::add(timeOfFrames, temp * buf.read_uint32(0, bigEndian)); } if (currentStream_ == Video) xmpData_["Xmp.video.FrameRate"] = (double)totalframes * (double)timeScale_ / (double)timeOfFrames;