Closed Bug 1561486 Opened 3 years ago Closed 3 years ago

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


(Core :: Fuzzing, defect, P1)




Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed


(Reporter: tsmith, Assigned: jya)



(Keywords: sec-other, Whiteboard: [post-critsmash-triage][adv-main70-])


(3 files)

Attached file 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/
    #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/
    #23 0x7fb33e702b9e in RunHandler ipc/chromium/src/base/
    #24 0x7fb33e702b9e in MessageLoop::Run() ipc/chromium/src/base/
    #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/
    #28 0x7fb36343f88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Attached video testcase.mp4
Assignee: nobody → jyavenard

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

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?

Flags: needinfo?(dveditz)

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.

Group: media-core-security → core-security-release
Flags: needinfo?(dveditz)
Keywords: sec-other

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(abillings)

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

Severity: normal → blocker
Flags: needinfo?(abillings)
Priority: -- → P1

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
Attachment #9075578 - Flags: sec-approval?
Attachment #9075578 - Flags: sec-approval? → sec-approval+

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

Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main70-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.