diff --git a/.github/workflows/on_PR_windows_matrix.yml b/.github/workflows/on_PR_windows_matrix.yml index 88acf0d1..06f0c913 100644 --- a/.github/workflows/on_PR_windows_matrix.yml +++ b/.github/workflows/on_PR_windows_matrix.yml @@ -8,6 +8,7 @@ on: pull_request: paths-ignore: - "*.md" + workflow_dispatch: push: branches: - main @@ -155,7 +156,7 @@ jobs: SHELLOPTS: igncr defaults: run: - shell: C:\tools\cygwin\bin\bash.exe -eo pipefail '{0}' + shell: C:\cygwin\bin\bash.exe -eo pipefail '{0}' steps: # Make sure we don't check out scripts using Windows CRLF line endings - run: git config --global core.autocrlf input @@ -163,7 +164,7 @@ jobs: - uses: actions/checkout@v3 - name: Set up Cygwin - uses: egor-tensin/setup-cygwin@v3 + uses: cygwin/cygwin-install-action@v3 with: platform: ${{matrix.platform}} packages: >- @@ -179,6 +180,7 @@ jobs: libbrotlicommon1 libbrotlidec1 libbrotli-devel + - name: Build run: | cmake -GNinja \ diff --git a/conanfile.py b/conanfile.py index 700028f3..65ff93cd 100644 --- a/conanfile.py +++ b/conanfile.py @@ -33,8 +33,6 @@ class Exiv2Conan(ConanFile): if self.options.unitTests: self.requires('gtest/1.12.1') - if self.settings.build_type == "Debug": - self.options['gtest'].debug_postfix = '' if self.options.xmp: self.requires('XmpSdk/2016.7@piponazo/stable') # from conan-piponazo 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/bmffimage.cpp b/src/bmffimage.cpp index 8aa38e8a..47bfe311 100644 --- a/src/bmffimage.cpp +++ b/src/bmffimage.cpp @@ -164,13 +164,30 @@ std::string BmffImage::uuidName(const Exiv2::DataBuf& uuid) { } #ifdef EXV_HAVE_BROTLI -void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBufSize, DataBuf& arr) { - BrotliDecoderState* decoder = NULL; - decoder = BrotliDecoderCreateInstance(NULL, NULL, NULL); - if (!decoder) { - throw Error(ErrorCode::kerMallocFailed); + +// Wrapper class for BrotliDecoderState that automatically calls +// BrotliDecoderDestroyInstance in its destructor. +class BrotliDecoderWrapper { + BrotliDecoderState* decoder_; + + public: + BrotliDecoderWrapper() : decoder_(BrotliDecoderCreateInstance(NULL, NULL, NULL)) { + if (!decoder_) { + throw Error(ErrorCode::kerMallocFailed); + } } + ~BrotliDecoderWrapper() { + BrotliDecoderDestroyInstance(decoder_); + } + + BrotliDecoderState* get() const { + return decoder_; + } +}; + +void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBufSize, DataBuf& arr) { + BrotliDecoderWrapper decoder; size_t uncompressedLen = compressedBufSize * 2; // just a starting point BrotliDecoderResult result; int dos = 0; @@ -184,7 +201,8 @@ void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBuf arr.alloc(uncompressedLen); available_out = uncompressedLen - total_out; next_out = arr.data() + total_out; - result = BrotliDecoderDecompressStream(decoder, &available_in, &next_in, &available_out, &next_out, &total_out); + result = + BrotliDecoderDecompressStream(decoder.get(), &available_in, &next_in, &available_out, &next_out, &total_out); if (result == BROTLI_DECODER_RESULT_SUCCESS) { arr.resize(total_out); } else if (result == BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT) { @@ -200,12 +218,10 @@ void BmffImage::brotliUncompress(const byte* compressedBuf, size_t compressedBuf throw Error(ErrorCode::kerFailedToReadImageData); } else { // something bad happened - throw Error(ErrorCode::kerErrorMessage, BrotliDecoderErrorString(BrotliDecoderGetErrorCode(decoder))); + throw Error(ErrorCode::kerErrorMessage, BrotliDecoderErrorString(BrotliDecoderGetErrorCode(decoder.get()))); } } while (result != BROTLI_DECODER_RESULT_SUCCESS); - BrotliDecoderDestroyInstance(decoder); - if (result != BROTLI_DECODER_RESULT_SUCCESS) { throw Error(ErrorCode::kerFailedToReadImageData); } diff --git a/src/canonmn_int.cpp b/src/canonmn_int.cpp index 1842b916..414dcf18 100644 --- a/src/canonmn_int.cpp +++ b/src/canonmn_int.cpp @@ -379,6 +379,7 @@ constexpr TagDetails canonModelId[] = {{0x00000412, "EOS M50 / Kiss M"}, {0x80000465, "EOS R10"}, {0x80000467, "PowerShot ZOOM"}, {0x80000468, "EOS M50 Mark II / Kiss M2"}, + {0x80000481, "EOS R6 Mark II"}, //{ (long int)tbd, "EOS Ra" }, {0x80000520, "EOS D2000C"}, {0x80000560, "EOS D6000C"}}; diff --git a/src/quicktimevideo.cpp b/src/quicktimevideo.cpp index b44b3683..00db1fc1 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,16 +1096,15 @@ 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); - totalframes += temp; + const uint64_t temp = buf.read_uint32(0, bigEndian); + 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; @@ -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() { diff --git a/src/tags_int.cpp b/src/tags_int.cpp index 0f17aae5..a35a7c21 100644 --- a/src/tags_int.cpp +++ b/src/tags_int.cpp @@ -2328,7 +2328,6 @@ bool isMakerIfd(IfdId ifdId) { } bool isExifIfd(IfdId ifdId) { - bool rc; switch (ifdId) { case IfdId::ifd0Id: case IfdId::exifId: @@ -2349,13 +2348,10 @@ bool isExifIfd(IfdId ifdId) { case IfdId::subImage9Id: case IfdId::subThumb1Id: case IfdId::panaRawId: - rc = true; - break; + return true; default: - rc = false; - break; + return false; } - return rc; } void taglist(std::ostream& os, IfdId ifdId) { @@ -2554,11 +2550,9 @@ std::ostream& printUcs2(std::ostream& os, const Value& value, const ExifData*) { // Strip trailing UCS-2 0-characters while (buf.size() >= 2) { - if (buf.read_uint8(buf.size() - 1) == 0 && buf.read_uint8(buf.size() - 2) == 0) { - buf.resize(buf.size() - 2); - } else { + if (buf.read_uint8(buf.size() - 1) != 0 || buf.read_uint8(buf.size() - 2) != 0) break; - } + buf.resize(buf.size() - 2); } std::string str(buf.c_str(), buf.size()); diff --git a/src/xmp.cpp b/src/xmp.cpp index db3e253b..f136f651 100644 --- a/src/xmp.cpp +++ b/src/xmp.cpp @@ -495,12 +495,10 @@ void XmpData::eraseFamily(XmpData::iterator& pos) { std::string key(pos->key()); std::vector keys; while (pos != xmpMetadata_.end()) { - if (Exiv2::Internal::startsWith(pos->key(), key)) { - keys.push_back(pos->key()); - pos++; - } else { + if (!Exiv2::Internal::startsWith(pos->key(), key)) break; - } + keys.push_back(pos->key()); + pos++; } // now erase the family! for (const auto& k : keys) { diff --git a/test/data/issue_2423_poc.mp4 b/test/data/issue_2423_poc.mp4 new file mode 100644 index 00000000..bf8171b6 Binary files /dev/null and b/test/data/issue_2423_poc.mp4 differ diff --git a/test/data/issue_2427_poc.jpg b/test/data/issue_2427_poc.jpg new file mode 100644 index 00000000..e42d8e10 Binary files /dev/null and b/test/data/issue_2427_poc.jpg differ 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/bugfixes/github/test_issue_2427.py b/tests/bugfixes/github/test_issue_2427.py new file mode 100644 index 00000000..113aa0c5 --- /dev/null +++ b/tests/bugfixes/github/test_issue_2427.py @@ -0,0 +1,13 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, check_no_ASAN_UBSAN_errors + +class issue_2427_BmffImage_brotliUncompress_memleak(metaclass=CaseMeta): + url = "https://github.com/Exiv2/exiv2/issues/2427" + filename = "$data_path/issue_2427_poc.jpg" + commands = ["$exiv2 $filename"] + retval = [1] + stderr = ["""$exiv2_exception_message $filename: +CL_SPACE +"""] + stdout = [""] diff --git a/tests/regression_tests/test_regression_allfiles.py b/tests/regression_tests/test_regression_allfiles.py index 25ba4c0e..4ce711c2 100644 --- a/tests/regression_tests/test_regression_allfiles.py +++ b/tests/regression_tests/test_regression_allfiles.py @@ -65,6 +65,8 @@ def get_valid_files(data_dir): "issue_2377_poc.mp4", "issue_2383_poc.mp4", "issue_2393_poc.mp4", + "issue_2423_poc.mp4", + "issue_2427_poc.jpg", "2018-01-09-exiv2-crash-001.tiff", "cve_2017_1000126_stack-oob-read.webp", "exiv2-bug1247.jpg",