New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774760 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference WRITE in media::mp4::TrackRunIterator::Init

Project Member Reported by ClusterFuzz, Oct 14 2017

Issue description

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

Fuzzer: libFuzzer_mediasource_MP4_FLAC_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x000000000000
Crash State:
  media::mp4::TrackRunIterator::Init
  media::mp4::MP4StreamParser::ParseMoof
  media::mp4::MP4StreamParser::ParseBox
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=508791:508824

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org pnangunoori@chromium.org
Components: Blink>Media
Labels: M-63 Test-Predator-Wrong
Owner: wolenetz@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using the code search for the file, “track_run_iterator.cc” assigning to concern owner from GIT revision log.
Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/01a1196369f223e75d1c2e1687397828ec346641
@wolenetz -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Thank You.

Hmm. Under debug ASAN, this is an OOM (malloc(9824930256)).
Cc: mmoroz@chromium.org
Components: -Blink>Media Internals>Media>Source
Eww. UBSAN-instrumented std::vector::resize() is hitting this internally.

cc+=mmoroz@ -- have you seen similar in other areas? Is there a safer way to do such a vector allocation?

Comment 4 by mmoroz@chromium.org, Oct 16 2017

Cc: och...@chromium.org mbarbe...@chromium.org kcc@chromium.org metzman@chromium.org
I don't recall seeing this before. CC'ing more folks to comment.


Friendly ping :)
This seems to not necessarily be a media product issue, rather a UBSAN infra sanitizer/library issue. Please help triage/re-assign.

Comment 6 by mmoroz@chromium.org, Oct 20 2017

Cc: glider@chromium.org p...@chromium.org
CCing a couple more folks who knows libc++ better. What should we do if we are getting null-deref from vector::resize call?

The crash reproduces locally as well, and the peak_rss_mb is 49, so it doesn't look like there is not enough memory on my 128GB RAM desktop.

Comment 7 by glider@chromium.org, Oct 23 2017

For the record I'm copying the report here (I had some problems accessing it):

UndefinedBehaviorSanitizer:DEADLYSIGNAL
==8475==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000030 (pc 0x000000952f93 bp 0x7ffdc29f17e0 sp 0x7ffdc29f17c0 T8475)
==8475==The signal is caused by a WRITE memory access.
==8475==Hint: address points to the zero page.
#0 0x952f92 in construct<media::mp4::SampleInfo> buildtools/third_party/libc++/trunk/include/memory:1798:31
#1 0x952f92 in __construct<media::mp4::SampleInfo> buildtools/third_party/libc++/trunk/include/memory:1709
#2 0x952f92 in construct<media::mp4::SampleInfo> buildtools/third_party/libc++/trunk/include/memory:1555
#3 0x952f92 in std::__1::__split_buffer<media::mp4::SampleInfo, std::__1::allocator<media::mp4::SampleInfo>&>::__construct_at_end(unsigned long) buildtools/third_party/libc++/trunk/include/__split_buffer:201
#4 0x952d9c in std::__1::vector<media::mp4::SampleInfo, std::__1::allocator<media::mp4::SampleInfo> >::__append(unsigned long) buildtools/third_party/libc++/trunk/include/vector:1042:13
#5 0x94f841 in media::mp4::TrackRunIterator::Init(media::mp4::MovieFragment const&) media/formats/mp4/track_run_iterator.cc:412:25
#6 0x94a491 in media::mp4::MP4StreamParser::ParseMoof(media::mp4::BoxReader*) media/formats/mp4/mp4_stream_parser.cc:552:3
#7 0x93ddad in media::mp4::MP4StreamParser::ParseBox(bool*) media/formats/mp4/mp4_stream_parser.cc:206:13
#8 0x93d24f in media::mp4::MP4StreamParser::Parse(unsigned char const*, int) media/formats/mp4/mp4_stream_parser.cc:159:18
#9 0xa17692 in media::SourceBufferState::Append(unsigned char const*, unsigned long, base::TimeDelta, base::TimeDelta, base::TimeDelta*) media/filters/source_buffer_state.cc:221:33
#10 0x9a7e33 in media::ChunkDemuxer::AppendData(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned char const*, unsigned long, base::TimeDelta, base::TimeDelta, base::TimeDelta*) media/filters/chunk_demuxer.cc:889:37
#11 0x60cd3c in media::MockMediaSource::AppendData(unsigned long) media/test/mock_media_source.cc:89:34
#12 0x60d560 in media::MockMediaSource::DemuxerOpenedTask() media/test/mock_media_source.cc:177:3
#13 0x61cc9c in void base::internal::Invoker<base::internal::BindState<void (media::MockMediaSource::*)(), base::internal::UnretainedWrapper<media::MockMediaSource> >, void ()>::RunImpl<void (media::MockMediaSource::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<media::MockMediaSource> > const&, 0ul>(void (media::MockMediaSource::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<media::MockMediaSource> > const&, std::__1::integer_sequence<unsigned long, 0ul>) base/bind_internal.h:349:12
#14 0x62864f in base::OnceCallback<void ()>::Run() && base/callback.h:64:12
#15 0xab4f24 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:57:33
#16 0xaa9dca in base::MessageLoop::RunTask(base::PendingTask*) base/message_loop/message_loop.cc:392:25
#17 0xaaa7af in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) base/message_loop/message_loop.cc:404:5
#18 0xaaaf23 in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:450:16
#19 0xab5bc5 in base::MessagePumpDefault::Run(base::MessagePump::Delegate*) base/message_loop/message_pump_default.cc:37:31
#20 0xaf608c in base::RunLoop::Run() base/run_loop.cc:118:14

Comment 8 by glider@chromium.org, Oct 23 2017

Off the top of my head I don't see what's wrong here.
Max, is the bug reproducible without UBSan? (E.g. with ASan?)

Comment 9 by mmoroz@chromium.org, Oct 23 2017

Running this with ASan gives a bit more sense:

$ ./mediasource_MP4_FLAC_pipeline_integration_fuzzer: Running 1 inputs 1 time(s) each.
Running: ./clusterfuzz-testcase-minimized-4965251492872192
==40976== ERROR: libFuzzer: out-of-memory (malloc(9824930256))
   To change the out-of-memory limit use -rss_limit_mb=<N>

    #0 0xa0c8f7 in __sanitizer_print_stack_trace (/usr/local/google/home/mmoroz/Projects/new/chromium/src/out/asan/mediasource_MP4_FLAC_pipeline_integration_fuzzer+0xa0c8f7)
    #1 0xaab171 in fuzzer::Fuzzer::HandleMalloc(unsigned long) third_party/libFuzzer/src/FuzzerLoop.cpp:105:5
    #2 0xaaafe6 in fuzzer::MallocHook(void const volatile*, unsigned long) third_party/libFuzzer/src/FuzzerLoop.cpp:79:6
    #3 0xa12b4d in __sanitizer::RunMallocHooks(void const*, unsigned long) (/usr/local/google/home/mmoroz/Projects/new/chromium/src/out/asan/mediasource_MP4_FLAC_pipeline_integration_fuzzer+0xa12b4d)

SUMMARY: libFuzzer: out-of-memory


9824930256 is 9+GB (9,824,930,256)


I've also tried running that with a greater rss_limit_mb value, it didn't crash but became a timeout (i.e. didn't finish in a couple of minutes).


I guess we should treat this as OOM? Is it OK that vector doesn't check the result of memory allocation and crashes with null-deref though?
Cc: dvyukov@chromium.org
+dvyukov
> I guess we should treat this as OOM? Is it OK that vector doesn't check the result of memory allocation and crashes with null-deref though?

According to Dima, this sounds valid, as the C++ allocator isn't expected to return NULL. If there were exceptions, the allocator would've thrown std::bad_alloc, but since there're none there's nothing the standard library can do to prevent the crash.
Thank you guys. Matt, it's probably worth to consider this as OOM and fix or prevent if possible.
For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md.

The link referenced in the description is no longer valid.
Cc: dalecur...@chromium.org wolenetz@chromium.org
Owner: sande...@chromium.org
OK. The underlying root cause of attempted large allocation is large trun.sample_count.

=> sandersd@ who is working on capping trun.sample_count prior to such allocations (e.g. related  bug 776721 ).
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 24 2017

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

commit 91965df34bca6d9236e62c7a228354d17843dcfa
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue Oct 24 23:29:56 2017

Limit TrackRunInfo.samples allocation size to 150MB.

When trun.sample_count is large, TrackRunInfo.samples may be allocated larger than available memory.
This CL caps that allocation to 150MB, and removes the earlier limit that disallowed two (or more)
zero-sized samples in a row.

This effectively limits the number of samples per trun to 6.5M, which also helps avoid timeouts in
fuzzing. That problem is unlikely to be solved by this CL alone, though.

Bug:  776721 , 776246 , 774760 , 776018 , 770577 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib4c35c066193a9da8ef460d9d9283e999d803ced
Reviewed-on: https://chromium-review.googlesource.com/734702
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511301}
[modify] https://crrev.com/91965df34bca6d9236e62c7a228354d17843dcfa/media/formats/mp4/track_run_iterator.cc

Project Member

Comment 15 by ClusterFuzz, Oct 25 2017

ClusterFuzz has detected this issue as fixed in range 511297:511319.

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

Fuzzer: libFuzzer_mediasource_MP4_FLAC_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x000000000000
Crash State:
  media::mp4::TrackRunIterator::Init
  media::mp4::MP4StreamParser::ParseMoof
  media::mp4::MP4StreamParser::ParseBox
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=508791:508824
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=511297:511319

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

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.
Project Member

Comment 16 by ClusterFuzz, Oct 25 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 4965251492872192 is verified as fixed, so closing issue as verified.

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

Sign in to add a comment