diff options
author | Michal Klocek <michal.klocek@qt.io> | 2017-05-24 12:35:12 +0200 |
---|---|---|
committer | Michal Klocek <michal.klocek@qt.io> | 2017-05-29 10:22:20 +0000 |
commit | defc5379f040c1dee5680418233c74e95492d43e (patch) | |
tree | c6aa797682f20877bcf46d1d97779204da6b4bd5 | |
parent | d540bef6bdab1a6356aa69947a76bc0cd7692c38 (diff) |
-- Cherry-pick notes --
This CL is a merge of the following cherry-picked commits:
d5e2e15 MSE: Fix moar mp4 parsing security bugs.
5041e28 MSE: Fix Mp4 SAIO parsing overflow
24f5635 MSE: Fix Mp4 TRUN parsing overflow
These each had conflicts due to dependency on safe_math.h functions
that are not present in this branch (base::CheckMul).
-- CL description --
Boxes with various sub-entries read the entry count from the user
provided mp4. Do not trust the counts. Check for size_t and vector
resize() overflow to avoid OOB writes in vector allocation.
Additionally, verify we have enough bytes to continue parsing before
allocating vectors to store parsed data.
Also evaluated other box_definition.cc vector resize() calls. Added
one additional check for SampleEncryptionEntry (probably overkill).
BUG=679645, 679646, 679647, 679653, 679640, 679641
TESTS=new unit tests, manual verification of PoCs
TBR=dalecurtis@chromium.org
Review-Url: https://codereview.chromium.org/2654913002 .
Cr-Commit-Position: refs/branch-heads/2924@{#857}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}
Change-Id: I6f1cf4e71e91cc8391a223750ef7190a0bb4481c
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r-- | chromium/media/formats/mp4/box_definitions.cc | 84 | ||||
-rw-r--r-- | chromium/media/formats/mp4/cenc.cc | 2 |
2 files changed, 70 insertions, 16 deletions
diff --git a/chromium/media/formats/mp4/box_definitions.cc b/chromium/media/formats/mp4/box_definitions.cc index fa76eb2b930..35a7c1913b3 100644 --- a/chromium/media/formats/mp4/box_definitions.cc +++ b/chromium/media/formats/mp4/box_definitions.cc @@ -105,8 +105,17 @@ bool SampleAuxiliaryInformationOffset::Parse(BoxReader* reader) { RCHECK(reader->SkipBytes(8)); uint32_t count; - RCHECK(reader->Read4(&count) && - reader->HasBytes(count * (reader->version() == 1 ? 8 : 4))); + RCHECK(reader->Read4(&count)); + int bytes_per_offset = reader->version() == 1 ? 8 : 4; + + base::CheckedNumeric<size_t> bytes_needed = + base::CheckedNumeric<size_t>(bytes_per_offset); + bytes_needed *= count; + RCHECK_MEDIA_LOGGED(bytes_needed.IsValid(), reader->media_log(), + "Extreme SAIO count exceeds implementation limit."); + RCHECK(reader->HasBytes(bytes_needed.ValueOrDie())); + + RCHECK(count <= offsets.max_size()); offsets.resize(count); for (uint32_t i = 0; i < count; i++) { @@ -338,11 +347,16 @@ bool EditList::Parse(BoxReader* reader) { uint32_t count; RCHECK(reader->ReadFullBoxHeader() && reader->Read4(&count)); - if (reader->version() == 1) { - RCHECK(reader->HasBytes(count * 20)); - } else { - RCHECK(reader->HasBytes(count * 12)); - } + const size_t bytes_per_edit = reader->version() == 1 ? 20 : 12; + + base::CheckedNumeric<size_t> bytes_needed = + base::CheckedNumeric<size_t>(bytes_per_edit); + bytes_needed *= count; + RCHECK_MEDIA_LOGGED(bytes_needed.IsValid(), reader->media_log(), + "Extreme ELST count exceeds implementation limit."); + RCHECK(reader->HasBytes(bytes_needed.ValueOrDie())); + + RCHECK(count <= edits.max_size()); edits.resize(count); for (std::vector<EditListEntry>::iterator edit = edits.begin(); @@ -855,16 +869,32 @@ bool TrackFragmentRun::Parse(BoxReader* reader) { 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) + const size_t bytes_per_field = 4; + + base::CheckedNumeric<size_t> bytes_needed(fields); + bytes_needed *= bytes_per_field; + bytes_needed *= sample_count; + RCHECK_MEDIA_LOGGED( + bytes_needed.IsValid(), reader->media_log(), + "Extreme TRUN sample count exceeds implementation limit."); + RCHECK(reader->HasBytes(bytes_needed.ValueOrDie())); + + 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) @@ -906,6 +936,16 @@ bool SampleToGroup::Parse(BoxReader* reader) { uint32_t count; RCHECK(reader->Read4(&count)); + + const size_t bytes_per_entry = 8; + base::CheckedNumeric<size_t> bytes_needed = + base::CheckedNumeric<size_t>(bytes_per_entry); + bytes_needed *= count; + RCHECK_MEDIA_LOGGED(bytes_needed.IsValid(), reader->media_log(), + "Extreme SBGP count exceeds implementation limit."); + RCHECK(reader->HasBytes(bytes_needed.ValueOrDie())); + + RCHECK(count <= entries.max_size()); entries.resize(count); for (uint32_t i = 0; i < count; ++i) { RCHECK(reader->Read4(&entries[i].sample_count) && @@ -944,6 +984,19 @@ bool SampleGroupDescription::Parse(BoxReader* reader) { uint32_t count; RCHECK(reader->Read4(&count)); + + // Check that we have at least two bytes for each entry before allocating a + // potentially huge entries vector. In reality each entry will require a + // variable number of bytes in excess of 2. + const int bytes_per_entry = 2; + base::CheckedNumeric<size_t> bytes_needed = + base::CheckedNumeric<size_t>(bytes_per_entry); + bytes_needed *= count; + RCHECK_MEDIA_LOGGED(bytes_needed.IsValid(), reader->media_log(), + "Extreme SGPD count exceeds implementation limit."); + RCHECK(reader->HasBytes(bytes_needed.ValueOrDie())); + + RCHECK(count <= entries.max_size()); entries.resize(count); for (uint32_t i = 0; i < count; ++i) { if (version == 1) { @@ -1024,9 +1077,10 @@ bool IndependentAndDisposableSamples::Parse(BoxReader* reader) { RCHECK(reader->version() == 0); RCHECK(reader->flags() == 0); - int sample_count = reader->size() - reader->pos(); + size_t sample_count = reader->size() - reader->pos(); + RCHECK(sample_count <= sample_depends_on_.max_size()); sample_depends_on_.resize(sample_count); - for (int i = 0; i < sample_count; ++i) { + for (size_t i = 0; i < sample_count; ++i) { uint8_t sample_info; RCHECK(reader->Read1(&sample_info)); diff --git a/chromium/media/formats/mp4/cenc.cc b/chromium/media/formats/mp4/cenc.cc index 3a7b59222e9..0d227093f35 100644 --- a/chromium/media/formats/mp4/cenc.cc +++ b/chromium/media/formats/mp4/cenc.cc @@ -29,7 +29,7 @@ bool FrameCENCInfo::Parse(int iv_size, BufferReader* reader) { uint16_t subsample_count; RCHECK(reader->Read2(&subsample_count) && reader->HasBytes(subsample_count * kEntrySize)); - + RCHECK(subsample_count <= subsamples.max_size()); subsamples.resize(subsample_count); for (int i = 0; i < subsample_count; i++) { uint16_t clear_bytes; |