1571037 - heap-use-after-free of BlobBodyStreamHolder

Status: VERIFIED FIXED

(Core :: DOM: File, defect, P1)

Description

[Tyson Smith [:tsmith]]

Attachment: test.zip

STR:

1. unpack test.zip
2. using a fuzzing build, a clean profile and the included prefs.js launch the browser
3. open launcher.html
4. wait 30 - 45 seconds

I can consistently reproduce the issue with a fuzzing ASan build.

Reproduced with m-c:
BuildID=20190731215544
SourceStamp=b0124f06562982dce60b820d95aad23afd5cec90

==30998==ERROR: AddressSanitizer: heap-use-after-free on address 0x6030006a0780 at pc 0x7fe298c69f91 bp 0x7ffd8f4de5f0 sp 0x7ffd8f4de5e8
READ of size 8 at 0x6030006a0780 thread T0 (file:// Content)
    #0 0x7fe298c69f90 in CanonicalizeXPCOMParticipant /src/xpcom/base/nsCycleCollector.cpp:848:8
    #1 0x7fe298c69f90 in CCGraphBuilder::NoteXPCOMChild(nsISupports*) /src/xpcom/base/nsCycleCollector.cpp:2155
    #2 0x7fe298c2c374 in mozilla::CycleCollectedJSRuntime::TraverseGCThing(mozilla::CycleCollectedJSRuntime::TraverseSelect, JS::GCCellPtr, nsCycleCollectionTraversalCallback&) /src/xpcom/base/CycleCollectedJSRuntime.cpp:707:5
    #3 0x7fe298c2bee7 in mozilla::JSGCThingParticipant::TraverseNative(void*, nsCycleCollectionTraversalCallback&) /src/xpcom/base/CycleCollectedJSRuntime.cpp:364:12
    #4 0x7fe298c6795a in TraverseNativeAndJS /src/xpcom/base/nsCycleCollectionParticipant.h:126:19
    #5 0x7fe298c6795a in CCGraphBuilder::BuildGraph(js::SliceBudget&) /src/xpcom/base/nsCycleCollector.cpp:2061
    #6 0x7fe298c70154 in nsCycleCollector::MarkRoots(js::SliceBudget&) /src/xpcom/base/nsCycleCollector.cpp:2672:33
    #7 0x7fe298c7706c in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3417:9
    #8 0x7fe298c7af04 in nsCycleCollector_collectSlice(js::SliceBudget&, bool) /src/xpcom/base/nsCycleCollector.cpp:3962:21
    #9 0x7fe29d18d851 in nsJSContext::RunCycleCollectorSlice(mozilla::TimeStamp) /src/dom/base/nsJSEnvironment.cpp:1479:3
    #10 0x7fe29d19255c in CCRunnerFired(mozilla::TimeStamp) /src/dom/base/nsJSEnvironment.cpp:1869:7
    #11 0x7fe298df2b60 in operator() /src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.4.0/../../../../include/c++/6.4.0/functional:2127:14
    #12 0x7fe298df2b60 in mozilla::IdleTaskRunner::Run() /src/xpcom/threads/IdleTaskRunner.cpp:58
    #13 0x7fe298e31dc0 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1224:14
    #14 0x7fe298e381d8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #15 0x7fe29a025baf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #16 0x7fe299f22c52 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #17 0x7fe299f22c52 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #18 0x7fe299f22c52 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #19 0x7fe2a216df89 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #20 0x7fe2a603b55f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:919:20
    #21 0x7fe299f22c52 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #22 0x7fe299f22c52 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #23 0x7fe299f22c52 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #24 0x7fe2a603ae06 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:754:34
    #25 0x55fcea761173 in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #26 0x55fcea761173 in main /src/browser/app/nsBrowserApp.cpp:267
    #27 0x7fe2ba61cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #28 0x55fcea6826ac in _start (/home/worker/builds/m-c-20190802094835-fuzzing-asan-opt/firefox+0x456ac)

0x6030006a0780 is located 0 bytes inside of 32-byte region [0x6030006a0780,0x6030006a07a0)
freed by thread T0 (file:// Content) here:
    #0 0x55fcea72dd42 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7fe298c90bc2 in MaybeKillObject /src/xpcom/base/nsCycleCollector.cpp:2429:29
    #2 0x7fe298c90bc2 in SnowWhiteKiller::Visit(nsPurpleBuffer&, nsPurpleBufferEntry*) /src/xpcom/base/nsCycleCollector.cpp:2459
    #3 0x7fe298c6e812 in void nsPurpleBuffer::VisitEntries<SnowWhiteKiller>(SnowWhiteKiller&) /src/xpcom/base/nsCycleCollector.cpp:941:23
    #4 0x7fe298c6fa59 in nsCycleCollector::FreeSnowWhiteWithBudget(js::SliceBudget&) /src/xpcom/base/nsCycleCollector.cpp:2624:14
    #5 0x7fe29b139c18 in AsyncFreeSnowWhite::Run() /src/js/xpconnect/src/XPCJSRuntime.cpp:146:9
    #6 0x7fe298e4ca3c in Run /src/xpcom/threads/nsThreadUtils.cpp:331:22
    #7 0x7fe298e4ca3c in IdleRunnableWrapper::TimedOut(nsITimer*, void*) /src/xpcom/threads/nsThreadUtils.cpp:337
    #8 0x7fe298e1f16b in nsTimerImpl::Fire(int) /src/xpcom/threads/nsTimerImpl.cpp:561:7
    #9 0x7fe298e1e9e9 in nsTimerEvent::Run() /src/xpcom/threads/TimerThread.cpp:260:11
    #10 0x7fe298e31dc0 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1224:14
    #11 0x7fe298e381d8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #12 0x7fe29a025baf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #13 0x7fe299f22c52 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #14 0x7fe299f22c52 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #15 0x7fe299f22c52 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #16 0x7fe2a216df89 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #17 0x7fe2a603b55f in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:919:20
    #18 0x7fe299f22c52 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #19 0x7fe299f22c52 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #20 0x7fe299f22c52 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #21 0x7fe2a603ae06 in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:754:34
    #22 0x55fcea761173 in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #23 0x55fcea761173 in main /src/browser/app/nsBrowserApp.cpp:267
    #24 0x7fe2ba61cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

previously allocated by thread T0 (file:// Content) here:
    #0 0x55fcea72e0c3 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x55fcea762e2d in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7fe2a0031301 in operator new /src/obj-firefox/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7fe2a0031301 in mozilla::dom::Blob::Stream(JSContext*, JS::MutableHandle<JSObject*>, mozilla::ErrorResult&) /src/dom/file/Blob.cpp:362
    #4 0x7fe29d42b951 in mozilla::dom::Blob_Binding::stream(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Blob*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/BlobBinding.cpp:745:24
    #5 0x7fe29f7bcf2d in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3163:13
    #6 0x7fe2a62f3857 in CallJSNative /src/js/src/vm/Interpreter.cpp:448:13
    #7 0x7fe2a62f3857 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:540
    #8 0x7fe2a62f6582 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:611:8
    #9 0x7fe2a6ee477f in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /src/js/src/proxy/Wrapper.cpp:162:10
    #10 0x7fe2a6e734e1 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /src/js/src/proxy/CrossCompartmentWrapper.cpp:237:19
    #11 0x7fe2a6ec3f6d in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /src/js/src/proxy/Proxy.cpp:504:19
    #12 0x7fe2a62f4a25 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:514:14
    #13 0x7fe2a62dc113 in CallFromStack /src/js/src/vm/Interpreter.cpp:599:10
    #14 0x7fe2a62dc113 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3084
    #15 0x7fe2a62bdd7f in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:425:10
    #16 0x7fe2a62fa11f in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /src/js/src/vm/Interpreter.cpp:787:13
    #17 0x7fe2a63e4789 in EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) /src/js/src/builtin/Eval.cpp:335:10
    #18 0x7fe2a63e68eb in js::DirectEval(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /src/js/src/builtin/Eval.cpp:449:10
    #19 0x7fe2a75a8d28 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /src/js/src/jit/BaselineIC.cpp:3196:10
    #20 0x201f0bca0797  (<unknown module>)
    #21 0x6250005328cf  (<unknown module>)
    #22 0x201f0bcb7bf6  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free /src/xpcom/base/nsCycleCollector.cpp:848:8 in CanonicalizeXPCOMParticipant
Shadow bytes around the buggy address:
  0x0c06800cc0a0: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x0c06800cc0b0: fd fa fa fa fd fd fd fd fa fa fd fd fd fa fa fa
  0x0c06800cc0c0: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fa
  0x0c06800cc0d0: fa fa fd fd fd fa fa fa fd fd fd fd fa fa fd fd
  0x0c06800cc0e0: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa
=>0x0c06800cc0f0:[fd]fd fd fd fa fa 00 00 05 fa fa fa fd fd fd fa
  0x0c06800cc100: fa fa fd fd fd fa fa fa fd fd fd fd fa fa fd fd
  0x0c06800cc110: fd fa fa fa fd fd fd fa fa fa fd fd fd fd fa fa
  0x0c06800cc120: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fd
  0x0c06800cc130: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd
  0x0c06800cc140: fd fd fa fa fd fd fd fa fa fa fd fd fd fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==30998==ABORTING

Comment 1

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Comment 2

[Andrew McCreight [:mccr8]]

Andrea, can you take a look please? Maybe there just needs to be call to DropJSObjects() in the dtor?

Comment 3

[Andrea Marchesini [:baku]]

On linux, I have hard time to test it because the test crashes here:
Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Paint() is not implemented!), at /home/baku/Sources/m/proxy/src/layout/painting/nsDisplayList.h:3202

https://searchfox.org/mozilla-central/rev/0ffa9e372df56c95547fed9c3433ddec4fbf6f11/layout/painting/nsDisplayList.h#3202

BTW, is the test.zip correct? It doesn't contain any blob.stream() call...

Comment 4

[Tyson Smith [:tsmith]]

Yes test.zip is correct. It is a seeded fuzzer not a true test case so the outcome is dependent on the build. Make sure to use an ASan opt fuzzing build with the included prefs.js file or it will likely not work or give different results (as seen above).

I can reproduce the issue consistently on Linux with m-c:
BuildID=20190805095413
SourceStamp=d681969e4480a2cad692be94adcbc2b861efb723

Comment 5

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

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

For more information, please visit auto_nag documentation.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Baku, do you think you'll have time to look at this or should I ? :)
(this is most probably a regression from the recent changes)

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Though, I've had no luck in reproducing this on an asan build.

Comment 8

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Baku, what's the next step here?

Comment 9

[Andrea Marchesini [:baku]]

I tried to reproduce it, but no luck. I still think the test uploaded is the wrong one, simply because, that test doesn't use blobs at all.
I would like to ask the reporter to kindly check again if he can reproduce the crash using the zip file here included.
I'll ping Tyson on slack too.

Comment 11

[Andrea Marchesini [:baku]]

Attachment: Bug 1571037 - BodyStreamHolder must have an extra ref in order to avoid being cycle-collected too soon, r?smaug

Comment 12

[Andrea Marchesini [:baku]]

Attachment: Bug 1571037 - BodyStreamHolder must release itself when unlinked, r?smaug

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 9092041 [details]
Bug 1571037 - BodyStreamHolder must release itself when unlinked, r?smaug

Security Approval Request

• How easily could an exploit be constructed based on the patch?: this is UAF. Not exploitable.
• 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?: 69
• If not all supported branches, which bug introduced the flaw?: Bug 1557121
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Easy to backport if needed.
• How likely is this patch to cause regressions; how much testing does it need?: low. It does only an extra cleanup.

Comment 14

[Al Billings [:abillings - ex-MoCo]]

Sec-approval+ for trunk. Please put a beta patch up and nominate it so we can avoid shipping the problem in any release.

Comment 15

[Andrea Marchesini [:baku]]

Comment on attachment 9092041 [details]
Bug 1571037 - BodyStreamHolder must release itself when unlinked, r?smaug

Beta/Release Uplift Approval Request

• User impact if declined: A crash can occur when Blob.stream() is used.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: There is a test included in the bug description
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We want to release BodyStream resources when the blob is unlinked. The code is simple enough to be low risk.
• String changes made/needed: none

Comment 17

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

https://hg.mozilla.org/integration/autoland/rev/661e87a948a05b15f9f9ddf84b6e81b1c0d26420
https://hg.mozilla.org/mozilla-central/rev/661e87a948a0

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9092041 [details]
Bug 1571037 - BodyStreamHolder must release itself when unlinked, r?smaug

Fixes a sec bug, approved for 70.0b8.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/89f6acce40bf2f2fffc3d49f21e219e9e8003123

Comment 21

[Bogdan Maris, Desktop Test Engineering]

I reproduced the initial issue using an old Nightly asan debug fuzz build from 2019-08-02, verified that latest Beta 70.0b8 asan fuzz and latest Nightly 71.0a1 asan fuzz does not show the issue anymore.