This crash was found using decoders new libfuzzer media fuzzing interface (bug 1465407)

==125399==ERROR: AddressSanitizer: heap-use-after-free on address 0x6110000c02c8 at pc 0x7fba744556df bp 0x7fb9ce593850 sp 0x7fb9ce593848
READ of size 8 at 0x6110000c02c8 thread T18 (MediaPD~oder #1)
    #0 0x7fba744556de in get objdir-ff-fuzzing/dist/include/mozilla/RefPtr.h:268:27
    #1 0x7fba744556de in operator-> objdir-ff-fuzzing/dist/include/mozilla/RefPtr.h:298
    #2 0x7fba744556de in mozilla::OpusDataDecoder::Init() dom/media/platforms/agnostic/OpusDecoder.cpp:62
    #3 0x7fba744ba790 in operator() dom/media/platforms/wrappers/AudioTrimmer.cpp:24:56
    #4 0x7fba744ba790 in mozilla::detail::ProxyFunctionRunnable<mozilla::AudioTrimmer::Init()::$_34, mozilla::MozPromise<mozilla::TrackInfo::TrackType, mozilla::MediaResult, true> >::Run() objdir-ff-fuzzing/dist/include/mozilla/MozPromise.h:1420
    #5 0x7fba6b00c7ab in mozilla::TaskQueue::Runner::Run() xpcom/threads/TaskQueue.cpp:199:12
    #6 0x7fba6b0500f4 in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:244:14
    #7 0x7fba6b0510a4 in non-virtual thunk to nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp
    #8 0x7fba6b043d40 in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1225:14
    #9 0x7fba6b04b644 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:486:10
    #10 0x7fba6c69379e in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:303:20
    #11 0x7fba6c502b9e in RunInternal ipc/chromium/src/base/message_loop.cc:315:10
    #12 0x7fba6c502b9e in RunHandler ipc/chromium/src/base/message_loop.cc:308
    #13 0x7fba6c502b9e in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:290
    #14 0x7fba6b03b718 in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:459:11
    #15 0x7fba92635f48 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:201:5
    #16 0x7fba922666da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #17 0x7fba9124488e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

This file will likely not reproduce the crash on a regular build. The libfuzzer fuzzing interface should be used to verify this bug. Feel free to ping me if needed.

The allocation and free stacks look the same as bug 1561490 -- could be same underlying bug?

In case these are related as per comment #2 it probably makes sense for you Alastor to look into this one as well.

Hi, Tyson,
As I couldn't reproduce this crash on my own build, may I know how to reproduce it and the crash in bug 1561490?
Thank you.

Hey Alastor, this uses the libfuzzer fuzzing interface[1]. To repro apply the patches from bug 1465407 and bug 1561256 and then follow the fuzzing build instructions[1]. Once you have the build to repro run MOZ_RUN_GTEST=1 LIBFUZZER=1 FUZZER=MediaOgg ./firefox <testcase>

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Fuzzing_Interface

(In reply to Daniel Veditz [:dveditz] from comment #2)

The allocation and free stacks look the same as bug 1561490 -- could be same underlying bug?

I believe it is.

Incorrect usage of the MediaDataDecoder.

I think the issue is in the Benchmark code.

I lower the priority however as this is not a problem that can occur in the wild.

Comment on attachment 9075300 [details]
Bug 1561484 - Keep MediaInfo object for entire benchmark. r?alwu

Security Approval Request

• How easily could an exploit be constructed based on the patch?: hard to tell. This code is typically not used in such fashion. It's only used with a set, static content that we control.

The new fuzzer access the underlying C++ code directly.

• 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?: 65
• 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?: should apply for all branches.
• How likely is this patch to cause regressions; how much testing does it need?: no possibility of regression

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Not a regression, but a problem caused by enabling an internal interface for fuzz testing only.

sec-approval+ for trunk.

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

(In reply to Nils Ohlmeier [:drno] from comment #11)

Not a regression, but a problem caused by enabling an internal interface for fuzz testing only.

Actually, the bug exists all the time. It's just quite hard to exploit as we only use it with our own static content (the VP9 sample).

Landed: https://hg.mozilla.org/integration/autoland/rev/b157ff9c0f229695504eec11632e1789a7820708

Backed out for failures with the build on most Linux machines (1564807): https://hg.mozilla.org/integration/autoland/rev/823177295f4c17856f7dacad1e1f3d4204d377ba

Relanded (this time the repository clones were aware of the changeset): https://hg.mozilla.org/integration/autoland/rev/ee059061515f19c66cbc1466108d2cf0e05de0bd

https://hg.mozilla.org/mozilla-central/rev/ee059061515f

Please request Beta and ESR68 approval on this when you get a chance. It grafts cleanly to both as-landed.

Comment on attachment 9075300 [details]
Bug 1561484 - Keep MediaInfo object for entire benchmark. r?alwu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: data race leading to use after free.
• Fix Landed on Version: 70
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We keep the object alive until it's no longer used
• String or UUID changes made by this patch: none

Beta/Release Uplift Approval Request

• User impact if declined: data race leading to use after free.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We keep the object alive until it's no longer used
• String changes made/needed: none

Comment on attachment 9075300 [details]
Bug 1561484 - Keep MediaInfo object for entire benchmark. r?alwu

Fixes a sec bug. Approved for 69.0b5 and 68.1esr.

https://hg.mozilla.org/releases/mozilla-beta/rev/8fcc3c00d7c0

https://hg.mozilla.org/releases/mozilla-esr68/rev/c9a3832408f8