Merge pull request #2424 from kevinbackhouse/fix-issue-2423

Fix long-running loop in QuickTimeVideo::sampleDesc

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
 

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;
+  uint32_t TagID = 0;
-  unsigned short dataLength = 0, dataType = 2;
+  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;
+  uint64_t totalframes = 0, timeOfFrames = 0;
-  noOfEntries = buf.read_uint32(0, bigEndian);
+  const uint32_t noOfEntries = buf.read_uint32(0, bigEndian);
-  size_t temp;
 
-  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;
+    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;
+  const uint32_t noOfEntries = buf.read_uint32(0, bigEndian);
-  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() {

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   = [""]

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",