Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically adding ccs based on suspected regression changelists:

ffmpeg: Fix memset size on ctts_data in mov_read_trun() by xhwang@chromium.org - https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/2aa88d702e397bf00f1f12aacec821a249512bdf

Revert "ffmpeg: Fix memset size on ctts_data in mov_read_trun()" by xhwang@chromium.org - https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/7eca0422a7ded44fd5262d2ba7609296cbbd211d

ffmpeg: Fix memset size on ctts_data in mov_read_trun() (round 2) by xhwang@chromium.org - https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/9ca66eafaf6fbbde11862e1468b365220ca142fd

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hmm, I cannot repro this issue on my Linux machine (rodete with some 'trusty' check manually disabled in third_party/instrumented_libraries/scripts/unpack_binaries.py).

Tried to RODO the tasks and see whether we can reliably repro.

tmathmeyer: Please help check whether you can repro on your Trusty machine. If yes, please help investigate this issue. If not, please assign back to me. Thanks!

I'm taking this one back -- I may need to use a Trusty VM.
I'll also relink it to the M67 FFmpeg roll crbug 803898 (though fix will likely need merging to M66).

With a Trusty docker image, I have what looks like a repro finally.

Great! Did you use https://sites.google.com/a/chromium.org/dev/developers/testing/memorysanitizer?pli=1#TOC-Running-on-other-distros-using-Docker ?

@#11 - Indeed I did - that was very helpful! Perhaps a quick primer on if/when/how to update/rebuild the image (it took on the order of an hour to build the image; I'd rather keep it if maintaining it/upgrading it is faster).

Investigating still. Intermediate results from the repro media:
track 0: video track:
stbl -> 1 stts entry, sample_count = 127
ctts entries: 123, but 1 is ignored due to count=0 duration=512
mov_read_ctts: ctts_count is 122
mov_build_index()'s logic to "Expand ctts entries such that we have a 1-1 mapping with samples":
ctts_count_old was 122, sample_count is 127
new ctts_count is ***126***, sample_count remains 127
--> This is not "1-1"
AVIndex logs for stream 0 show 127 samples (0-126)

track 1: audio track:
2 stts entries, sample counts 1+197 = 198
no "mov_read_ctts" called, ctts_count 0
mov_build_index therefore doesn't try to expand ctts entries (no ctts_data_old)
result of mov_build_index:
AVIndex logs for stream 1 show 198 samples
ctts_count:0 sample_count:198

track 2: video track
stbl -> 1 stts entry, sample_count = 127
ctts entries: 123, none ignored
mov_read_ctts: ctts_count is 123
mov_build_index()'s logic to "Expand ctts entries such that we have a 1-1 mapping with samples":
ctts_count_old was 123, sample_count is 127
new ctts_count is ***127***, sample_count remains 127
--> Unlike track 0, this **is** "1-1"
AVIndex logs for stream 2 show 127 samples (0-126)

track 3: audio track:
2 stts entries, sample counts 1+197 = 198
no "mov_read_ctts" called, ctts_count 0
mov_build_index therefore doesn't try to expand ctts entries (no ctts_data_old)
result of mov_build_index:
AVIndex logs for stream 3 show 198 samples
ctts_count:0 sample_count:198

Then some mvex,mdat,moof parsing logs followed by:
AVIndex logs for stream 0 now showing 377 samples (128-377 shown)

Then some logs related to calculating start times for each of the 4 tracks.

Then a bunch of mov_read_packets, one of which hits uninitialized while working on a track having a ctts_count of 377, with ctts_index at 126. allocation size is well over that (404), so this is not out-of-bounds. ctts_data for the index: duration is 0, count is 0, then uninitialized access detected (perhaps the instrumentation isn't precise enough - I'd expect my extra logging of that data to have hit the sanitizer on dref'ing that one.

10k' view, comparing the processing of track0 and track2: it looks like an ignored ctts entry in mov_read_ctts later causes failure of 1-1 ctts index <-> samples in mov_build_index, later leading to access of uninitialized info when attempting to retrieve ctts info in mov_read_packet.

Also, enabling TRACE loglevel in //media/base/media.cc prevents successful repro for this particular case. But specific fprintf(stderr,...) in the ffmpeg code got close enough to get the data in #14.

+Dale as FYI (this is RBS-66).

Possibly related recent changes: see https://bugs.chromium.org/p/chromium/issues/detail?id=812567#c26

Downstream fix is in review: https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/947110

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/ef99a5d2520f934fa6c74ef7219aaa47e8717914

commit ef99a5d2520f934fa6c74ef7219aaa47e8717914
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Fri Mar 02 23:22:20 2018

ffmpeg: Initialize a potential gap in ctts_data in mov_build_index

mov_read_ctts ignores ctts entries having count <= 0. Generally, the
aggregate of all ctts entries' count fields resulting from mov_read_ctts
can be less than the corresponding sample_count.

mov_build_index attempts to normalize any existing ctts_data counts to
be 1, to make a 1-1 mapping of a ctts_data entry to a sample.

That 1-1 mapping left a tail of uninitialized ctts_data entries when the
aggregate, normalized ctts_count < sample_count.

Even more generally, later usage of ctts_data may depend on the entire
ctts_allocated_size having been initialized.

This change memsets the entire allocation of the normalized ctts_data in
mov_build_index, to prevent use of uninitialized data later.

BUG= 816787 

Change-Id: I7fd7db255e3aeed076ee32c90cb2df211741c052
Reviewed-on: https://chromium-review.googlesource.com/947110
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>

[modify] https://crrev.com/ef99a5d2520f934fa6c74ef7219aaa47e8717914/libavformat/mov.c
[modify] https://crrev.com/ef99a5d2520f934fa6c74ef7219aaa47e8717914/chromium/patches/README

I've sent #19 upstream: https://patchwork.ffmpeg.org/patch/7798/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d063a3e1dec3f895e244ffad122dbe32399966d6

commit d063a3e1dec3f895e244ffad122dbe32399966d6
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Sat Mar 03 01:25:33 2018

Roll src/third_party/ffmpeg/ 9ca66eafa..ef99a5d25 (2 commits)

https://chromium.googlesource.com/chromium/third_party/ffmpeg.git/+log/9ca66eafaf6f..ef99a5d2520f

$ git log 9ca66eafa..ef99a5d25 --date=short --no-merges --format='%ad %ae %s'
2018-03-02 wolenetz ffmpeg: Initialize a potential gap in ctts_data in mov_build_index
2018-02-26 dalecurtis Revert "Change the way the ffmpeg_branding gets its defaults."

Created with:
  roll-dep src/third_party/ffmpeg
BUG= 816787 , 570754 , 795935 

Change-Id: I44bcdd293b786cdc4e754f81e82695578b0ba6c3
Reviewed-on: https://chromium-review.googlesource.com/947386
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540707}
[modify] https://crrev.com/d063a3e1dec3f895e244ffad122dbe32399966d6/DEPS

Pending CF verification of this being fixed on trunk, and it baking in Canary a few days, I'll request merge to M66 (which would be a low-risk ffmpeg DEPS (buildspec) change in the M66 branch) of #19 (but not dalecurtis' commit I rolled into trunk in #21).

ClusterFuzz has detected this issue as fixed in range 540702:540708.

Detailed report: https://clusterfuzz.com/testcase?key=6306464579452928

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  mov_read_packet
  ff_read_packet
  read_frame_internal
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=538981:538986
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=540702:540708

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6306464579452928

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6306464579452928 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

The NextAction date has arrived: 2018-03-05

Requesting merge to M66 (see #22).

This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Upstream has taken my change verbatim:  https://github.com/FFmpeg/FFmpeg/commit/133ddd38750acc01d0a9599d5b31375d33798d67

I'll remove the corresponding downstream patches/README entry in my M67 roll ( bug 803898 ).

--

Note: this patch is currently pending approval for merge to M66 (simple buildspec change).

+awhalley@ for M66 merge review

Friendly ping on merge request's review.

Does this not meet the bar for M66 merge?

Thanks for the ping.

abdulsyed - good for 66

Approving merge to M66 branch 3359 based on comment #33. Please merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/third_party/ffmpeg/+/c44f90ded0cc16a55a7be327470867abaa284c16

commit c44f90ded0cc16a55a7be327470867abaa284c16
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Mon Mar 12 20:15:41 2018

To M66: ffmpeg: Initialize a potential gap in ctts_data in mov_build_index

mov_read_ctts ignores ctts entries having count <= 0. Generally, the
aggregate of all ctts entries' count fields resulting from mov_read_ctts
can be less than the corresponding sample_count.

mov_build_index attempts to normalize any existing ctts_data counts to
be 1, to make a 1-1 mapping of a ctts_data entry to a sample.

That 1-1 mapping left a tail of uninitialized ctts_data entries when the
aggregate, normalized ctts_count < sample_count.

Even more generally, later usage of ctts_data may depend on the entire
ctts_allocated_size having been initialized.

This change memsets the entire allocation of the normalized ctts_data in
mov_build_index, to prevent use of uninitialized data later.

BUG= 816787 

Reviewed-on: https://chromium-review.googlesource.com/947110
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
(cherry picked from commit ef99a5d2520f934fa6c74ef7219aaa47e8717914)

TBR=xhwang@chromium.org

Change-Id: I367eb5aa7d1959d4d5dc178aa5ee8418461c64eb
Reviewed-on: https://chromium-review.googlesource.com/959462
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>

[modify] https://crrev.com/c44f90ded0cc16a55a7be327470867abaa284c16/libavformat/mov.c
[modify] https://crrev.com/c44f90ded0cc16a55a7be327470867abaa284c16/chromium/patches/README

If nothing is pending for M66, pls removed "Merge-Approved-66" label.

chromium ffmpeg branch-m66 created
#19 cherry-picked into that branch @ https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/959462

M66 buildspec not yet updated, so undoing the merge-merged-... label

Pls merge your change to M66 branch 3359 ASAP so we can pick it up for this week M66 Beta Promotion release. 

If it is already merged to M66, pls remove "Merge=Approved-66" label. Thank you.

M66 buildspec review is pending @ https://chrome-internal-review.googlesource.com/c/chrome/tools/buildspec/+/586660

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/c2026f79cfa95d5eb1b4769210f255b655916b78

commit c2026f79cfa95d5eb1b4769210f255b655916b78
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Mon Mar 12 21:57:57 2018

(#40 completed the merge to M66 of the fix for this issue)

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot