[media] Check for overflow in TrackRunIterator::cts().

This CL moves the CTS computation to AdvanceSample(), which now returns
a bool. Callers have been updated to fail when AdvanceSample() fails.

Bug: 772874
Change-Id: I1215f6a259eff310b964e1899be4e4138ff34087
Reviewed-on: https://chromium-review.googlesource.com/722164Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509664}

media/formats/mp4/mp4_stream_parser.cc

@@ -618,7 +618,9 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
   }
 
   if (!runs_->IsSampleValid()) {
-    runs_->AdvanceRun();
+    *err = !runs_->AdvanceRun();
+    if (*err)
+      return false;
     return true;
   }
 
@@ -636,7 +638,9 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
 
   // Skip this entire track if it's not one we're interested in
   if (!audio && !video) {
-    runs_->AdvanceRun();
+    *err = !runs_->AdvanceRun();
+    if (*err)
+      return false;
     return true;
   }
 
@@ -674,7 +678,9 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
     LIMITED_MEDIA_LOG(DEBUG, media_log_, num_empty_samples_skipped_,
                       kMaxEmptySampleLogs)
         << "Skipping 'trun' sample with size of 0.";
-    runs_->AdvanceSample();
+    *err = !runs_->AdvanceSample();
+    if (*err)
+      return false;
     return true;
   }
 
@@ -757,7 +763,7 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
   if (runs_->cts() != kNoTimestamp) {
     stream_buf->set_timestamp(runs_->cts());
   } else {
-    MEDIA_LOG(ERROR, media_log_) << "Frame CTS exceeds representable limit";
+    MEDIA_LOG(ERROR, media_log_) << "Frame PTS exceeds representable limit";
     *err = true;
     return false;
   }
@@ -779,7 +785,9 @@ bool MP4StreamParser::EnqueueSample(BufferQueueMap* buffers, bool* err) {
            << ", size=" << sample_size;
 
   (*buffers)[runs_->track_id()].push_back(stream_buf);
-  runs_->AdvanceSample();
+  *err = !runs_->AdvanceSample();
+  if (*err)
+    return false;
   return true;
 }
 
@@ -848,10 +856,11 @@ bool MP4StreamParser::ComputeHighestEndOffset(const MovieFragment& moof) {
       int64_t sample_end_offset = runs.sample_offset() + runs.sample_size();
       if (sample_end_offset > highest_end_offset_)
         highest_end_offset_ = sample_end_offset;
-
-      runs.AdvanceSample();
+      if (!runs.AdvanceSample())
+        return false;
     }
-    runs.AdvanceRun();
+    if (!runs.AdvanceRun())
+      return false;
   }
 
   return true;

media/formats/mp4/track_run_iterator.cc

@@ -10,6 +10,7 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/numerics/checked_math.h"
 #include "media/base/timestamp_constants.h"
 #include "media/formats/mp4/rcheck.h"
 #include "media/formats/mp4/sample_to_group_iterator.h"
@@ -149,12 +150,13 @@ static bool PopulateSampleInfo(const TrackExtends& trex,
     sample_info->duration = trex.default_sample_duration;
   }
 
-  if (i < trun.sample_composition_time_offsets.size()) {
-    sample_info->cts_offset = trun.sample_composition_time_offsets[i];
-  } else {
-    sample_info->cts_offset = 0;
+  auto cts_offset = -base::CheckedNumeric<int64_t>(edit_list_offset);
+  if (i < trun.sample_composition_time_offsets.size())
+    cts_offset += trun.sample_composition_time_offsets[i];
+  if (!cts_offset.AssignIfValid(&sample_info->cts_offset)) {
+    MEDIA_LOG(ERROR, media_log) << "PTS offset exceeds representable range.";
+    return false;
   }
-  sample_info->cts_offset += edit_list_offset;
 
   uint32_t flags;
   if (i < trun.sample_flags.size()) {
@@ -316,7 +318,7 @@ bool TrackRunIterator::Init(const MovieFragment& moof) {
       if (edits[0].media_time < 0) {
         DVLOG(1) << "Empty edit list entry ignored.";
       } else {
-        edit_list_offset = -edits[0].media_time;
+        edit_list_offset = edits[0].media_time;
       }
     }
 
@@ -486,27 +488,48 @@ bool TrackRunIterator::Init(const MovieFragment& moof) {
 
   std::sort(runs_.begin(), runs_.end(), CompareMinTrackRunDataOffset());
   run_itr_ = runs_.begin();
-  ResetRun();
+  return ResetRun();
+}
+
+bool TrackRunIterator::UpdateCts() {
+  // TODO(sandersd): Should |sample_cts_| be cleared in this case?
+  if (!IsSampleValid())
+    return true;
+  auto cts = base::CheckAdd(sample_dts_, sample_itr_->cts_offset);
+  if (!cts.AssignIfValid(&sample_cts_)) {
+    MEDIA_LOG(ERROR, media_log_) << "Sample PTS exceeds representable range.";
+    return false;
+  }
   return true;
 }
 
-void TrackRunIterator::AdvanceRun() {
+bool TrackRunIterator::AdvanceRun() {
   ++run_itr_;
-  ResetRun();
+  return ResetRun();
 }
 
-void TrackRunIterator::ResetRun() {
-  if (!IsRunValid()) return;
+bool TrackRunIterator::ResetRun() {
+  // TODO(sandersd): Should we clear all the values if the run is not valid?
+  if (!IsRunValid())
+    return true;
   sample_dts_ = run_itr_->start_dts;
   sample_offset_ = run_itr_->sample_start_offset;
   sample_itr_ = run_itr_->samples.begin();
+  // UpdateCts() must run after |sample_itr_| is updated to the current run.
+  return UpdateCts();
 }
 
-void TrackRunIterator::AdvanceSample() {
+bool TrackRunIterator::AdvanceSample() {
   DCHECK(IsSampleValid());
-  sample_dts_ += sample_itr_->duration;
+  auto dts = base::CheckAdd(sample_dts_, sample_itr_->duration);
+  if (!dts.AssignIfValid(&sample_dts_)) {
+    MEDIA_LOG(ERROR, media_log_) << "Sample DTS exceeds representable range.";
+    return false;
+  }
   sample_offset_ += sample_itr_->size;
   ++sample_itr_;
+  // UpdateCts() must run after |sample_itr_| is updated to the current sample.
+  return UpdateCts();
 }
 
 // This implementation only indicates a need for caching if CENC auxiliary
@@ -638,8 +661,7 @@ DecodeTimestamp TrackRunIterator::dts() const {
 
 base::TimeDelta TrackRunIterator::cts() const {
   DCHECK(IsSampleValid());
-  return TimeDeltaFromRational(sample_dts_ + sample_itr_->cts_offset,
-                               run_itr_->timescale);
+  return TimeDeltaFromRational(sample_cts_, run_itr_->timescale);
 }
 
 base::TimeDelta TrackRunIterator::duration() const {

media/formats/mp4/track_run_iterator.h

@@ -47,10 +47,12 @@ class MEDIA_EXPORT TrackRunIterator {
   bool IsRunValid() const;
   bool IsSampleValid() const;
 
-  // Advance the properties to refer to the next run or sample. Requires that
-  // the current sample be valid.
-  void AdvanceRun();
-  void AdvanceSample();
+  // Advance the properties to refer to the next run or sample. These return
+  // |false| on failure, but note that advancing to the end (IsRunValid() or
+  // IsSampleValid() return false) is not a failure, and the properties are not
+  // guaranteed to be consistent in that case.
+  bool AdvanceRun();
+  bool AdvanceSample();
 
   // Returns true if this track run has auxiliary information and has not yet
   // been cached. Only valid if IsRunValid().
@@ -91,7 +93,8 @@ class MEDIA_EXPORT TrackRunIterator {
   std::unique_ptr<DecryptConfig> GetDecryptConfig();
 
  private:
-  void ResetRun();
+  bool UpdateCts();
+  bool ResetRun();
   const TrackEncryption& track_encryption() const;
 
   uint32_t GetGroupDescriptionIndex(uint32_t sample_index) const;
@@ -112,6 +115,7 @@ class MEDIA_EXPORT TrackRunIterator {
   std::vector<SampleInfo>::const_iterator sample_itr_;
 
   int64_t sample_dts_;
+  int64_t sample_cts_;
   int64_t sample_offset_;
 
   DISALLOW_COPY_AND_ASSIGN(TrackRunIterator);