MSE: Fix Mp4 TRUN parsing overflow

media/formats/mp4/box_definitions.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/formats/mp4/box_definitions.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/numerics/safe_math.h"
 #include "base/strings/string_number_conversions.h"
 #include "media/base/media_switches.h"
 #include "media/base/video_types.h"
 #include "media/base/video_util.h"
 #include "media/filters/h264_parser.h"
 #include "media/formats/mp4/avc.h"
 #include "media/formats/mp4/es_descriptor.h"
 #include "media/formats/mp4/rcheck.h"
 #include "media/media_features.h"
 
@@ skipping 1093 matching lines @@
   } else {
     data_offset = 0;
   }
 
   uint32_t first_sample_flags = 0;
   if (first_sample_flags_present)
     RCHECK(reader->Read4(&first_sample_flags));
 
   int fields = sample_duration_present + sample_size_present +
       sample_flags_present + sample_composition_time_offsets_present;
-  RCHECK(reader->HasBytes(fields * sample_count));
 
-  if (sample_duration_present)
+  // |bytes_needed| is potentially 64-bit. Cast |sample_count| from uint32_t to
+  // size_t to avoid multiplication overflow.
+  base::CheckedNumeric<size_t> bytes_needed =
+      base::CheckMul(fields, static_cast<size_t>(sample_count));
+  RCHECK_MEDIA_LOGGED(bytes_needed.IsValid(), reader->media_log(),
+                      "Extreme TRUN sample count exceeds system address space");
+  RCHECK(reader->HasBytes(bytes_needed.ValueOrDie()));

[DaleCurtis, 2017/01/18 21:21:32]

Isn't this unreachable if IsValid() fails above?

[chcunningham, 2017/01/18 21:52:06]

Yes.

Its only reachable when IsValid passes, in which case we still want to check
HasBytes.

The "Die" part of ValueOrDie() is not reachable, but I think its better than
ValueOrDefault() (only two options AFAIK).

[DaleCurtis, 2017/01/18 21:53:04]

Whoops, sorry I totally skipped over the function call when analyzing.

+
+  if (sample_duration_present) {
+    RCHECK(sample_count <= sample_durations.max_size());
     sample_durations.resize(sample_count);
-  if (sample_size_present)
+  }
+  if (sample_size_present) {
+    RCHECK(sample_count <= sample_sizes.max_size());
     sample_sizes.resize(sample_count);
-  if (sample_flags_present)
+  }
+  if (sample_flags_present) {
+    RCHECK(sample_count <= sample_flags.max_size());
     sample_flags.resize(sample_count);
-  if (sample_composition_time_offsets_present)
+  }
+  if (sample_composition_time_offsets_present) {
+    RCHECK(sample_count <= sample_composition_time_offsets.max_size());
     sample_composition_time_offsets.resize(sample_count);
+  }
 
   for (uint32_t i = 0; i < sample_count; ++i) {
     if (sample_duration_present)
       RCHECK(reader->Read4(&sample_durations[i]));
     if (sample_size_present)
       RCHECK(reader->Read4(&sample_sizes[i]));
     if (sample_flags_present)
       RCHECK(reader->Read4(&sample_flags[i]));
     if (sample_composition_time_offsets_present)
       RCHECK(reader->Read4s(&sample_composition_time_offsets[i]));
@@ skipping 198 matching lines @@
 SampleDependsOn IndependentAndDisposableSamples::sample_depends_on(
     size_t i) const {
   if (i >= sample_depends_on_.size())
     return kSampleDependsOnUnknown;
 
   return sample_depends_on_[i];
 }
 
 }  // namespace mp4
 }  // namespace media