1561486 - heap-use-after-free in [@ mozilla::FuzzMediaResource::ReadAt]

Status: RESOLVED FIXED

(Core :: Fuzzing, defect, P1)

Description

[Tyson Smith [:tsmith]]

Attachment: full_log.txt

This looks more like a potential bug in the harness.

==125508==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000073a80 at pc 0x55dd4697baa9 bp 0x7fb32a873f90 sp 0x7fb32a873740
READ of size 8 at 0x61d000073a80 thread T22 (MediaPl~back #2)
    #0 0x55dd4697baa8 in __asan_memcpy /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:23:3
    #1 0x7fb34eb1ed1a in mozilla::FuzzMediaResource::ReadAt(long, char*, unsigned int, unsigned int*) dom/media/fuzz/FuzzMediaResource.cpp:40:5
    #2 0x7fb3461686ae in UncachedReadAt dom/media/MediaResource.cpp:346:21
    #3 0x7fb3461686ae in mozilla::MediaResourceIndex::ReadAt(long, char*, unsigned int, unsigned int*) dom/media/MediaResource.cpp:85
    #4 0x7fb346b449aa in mozilla::ResourceStream::ReadAt(long, void*, unsigned long, unsigned long*) dom/media/mp4/ResourceStream.cpp:27:29
    #5 0x7fb346ae9afd in mozilla::Box::Box(mozilla::BoxContext*, unsigned long, mozilla::Box const*) dom/media/mp4/Box.cpp:66:27
    #6 0x7fb346af7e73 in mozilla::MoofParser::BlockingReadNextMoof() dom/media/mp4/MoofParser.cpp:168:12
    #7 0x7fb346af6dd1 in mozilla::SampleIterator::Get() dom/media/mp4/Index.cpp:335:33
    #8 0x7fb346b19877 in Seek dom/media/mp4/Index.cpp:357:22
    #9 0x7fb346b19877 in mozilla::MP4TrackDemuxer::Reset() dom/media/mp4/MP4Demuxer.cpp:540
    #10 0x7fb345dd2a54 in mozilla::BenchmarkPlayback::FinalizeShutdown() dom/media/Benchmark.cpp:263:20
    #11 0x7fb345def4b9 in operator() dom/media/Benchmark.cpp:287:51
    #12 0x7fb345def4b9 in InvokeMethod<(lambda at dom/media/Benchmark.cpp:287:35), void ((lambda at dom/media/Benchmark.cpp:287:35)::*)() const, const bool &> objdir-ff-fuzzing/dist/include/mozilla/MozPromise.h:511
    #13 0x7fb345def4b9 in InvokeCallbackMethod<false, (lambda at dom/media/Benchmark.cpp:287:35), void ((lambda at dom/media/Benchmark.cpp:287:35)::*)() const, const bool &, RefPtr<mozilla::MozPromise<bool, bool, false>::Private> > objdir-ff-fuzzing/dist/include/mozilla/MozPromise.h:535
    #14 0x7fb345def4b9 in mozilla::MozPromise<bool, bool, false>::ThenValue<mozilla::BenchmarkPlayback::GlobalShutdown()::$_10::operator()() const::'lambda'(), mozilla::BenchmarkPlayback::GlobalShutdown()::$_10::operator()() const::'lambda0'()>::DoResolveOrRejectInternal(mozilla::MozPromise<bool, bool, false>::ResolveOrRejectValue&) objdir-ff-fuzzing/dist/include/mozilla/MozPromise.h:717
    #15 0x7fb33d2196bd in mozilla::MozPromise<bool, bool, false>::ThenValueBase::ResolveOrRejectRunnable::Run() objdir-ff-fuzzing/dist/include/mozilla/MozPromise.h:393:21
    #16 0x7fb33d20c7ab in mozilla::TaskQueue::Runner::Run() xpcom/threads/TaskQueue.cpp:199:12
    #17 0x7fb33d2500f4 in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:244:14
    #18 0x7fb33d2510a4 in non-virtual thunk to nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp
    #19 0x7fb33d243d40 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1225:14
    #20 0x7fb33d24b644 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:486:10
    #21 0x7fb33e8939ab in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:333:5
    #22 0x7fb33e702b9e in RunInternal ipc/chromium/src/base/message_loop.cc:315:10
    #23 0x7fb33e702b9e in RunHandler ipc/chromium/src/base/message_loop.cc:308
    #24 0x7fb33e702b9e in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:290
    #25 0x7fb33d23b718 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:459:11
    #26 0x7fb364830f48 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:201:5
    #27 0x7fb3644616da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #28 0x7fb36343f88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[Tyson Smith [:tsmith]]

Attachment: testcase.mp4

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1561486 - shutdown demuxer early. r?alwu

We don't need the demuxer after we've resolved the promise.

Comment 3

[Jean-Yves Avenard [:jya]]

This isn't a security bug. There's no possibility to use the code in such fashion. The problem was exposed with the new fuzzing wrapper.

Can we remove the security flag?

Comment 4

[Christian Holler (:decoder)]

I would prefer to keep the bug locked until bug 1465407 has received enough testing for us and landed (not because this bug is s-s but because it draws attention to what we are currently attempting to test).

I will look into review comments of bug 1465407 this week (thanks Jean-Yves for taking a look so quickly!) and I guess with the patch here landed, Tyson can meanwhile test more of these targets locally so we don't 0-day ourselves when landing the targets. This error here didn't happen locally by the way, when I wrote the targets (that was about a year ago). Maybe something changed meanwhile that exposed it.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:abillings, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Christian Holler (:decoder)]

This is a P1 for fuzzing because it blocks all media fuzzing with the new target.

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 9075578 [details]
Bug 1561486 - shutdown demuxer early. r?alwu

Security Approval Request

• How easily could an exploit be constructed based on the patch?: can't be as the code isn't exploited. Only asking for sec-approval as this bug is incorrectly marked as security sensitive.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?:
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: will apply safely
• How likely is this patch to cause regressions; how much testing does it need?: none

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/9b3f4d1a7c206b88fd3cf4ae8beb4cf145a3136e
https://hg.mozilla.org/mozilla-central/rev/9b3f4d1a7c20

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Doesn't sound like we need to worry about backporting this anywhere.