1439022 - heap-use-after-free in [@ OnBodyParseEnd]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tyson Smith [:tsmith]]

Found in mozilla-central 20180216-c4d818c13868

I am working on a testcase now.

==2878==ERROR: AddressSanitizer: heap-use-after-free on address 0x61700022f0a5 at pc 0x7fa8643532ed bp 0x7ffc16fb7560 sp 0x7ffc16fb7558
WRITE of size 1 at 0x61700022f0a5 thread T0
    #0 0x7fa8643532ec in OnBodyParseEnd /src/dom/xhr/XMLHttpRequestMainThread.cpp:2269:18
    #1 0x7fa8643532ec in mozilla::dom::nsXHRParseEndListener::HandleEvent(nsIDOMEvent*) /src/dom/xhr/XMLHttpRequestMainThread.h:873
    #2 0x7fa8629e0fd1 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1111:51
    #3 0x7fa8629e26ce in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1286:20
    #4 0x7fa8629cbf27 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:527:16
    #5 0x7fa8629cc5cc in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:590:5
    #6 0x7fa8629cfc83 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:915:9
    #7 0x7fa8629d1f7c in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /src/dom/events/EventDispatcher.cpp:994:12
    #8 0x7fa8601952ff in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /src/dom/base/nsINode.cpp:1274:5
    #9 0x7fa85fca77fe in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool, bool*, bool) /src/dom/base/nsContentUtils.cpp:4587:18
    #10 0x7fa85fca75b4 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool*) /src/dom/base/nsContentUtils.cpp:4555:10
    #11 0x7fa8600a5631 in nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5365:3
    #12 0x7fa86010b0d4 in applyImpl<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1149:12
    #13 0x7fa86010b0d4 in apply<nsDocument, void (nsDocument::*)()> /src/obj-firefox/dist/include/nsThreadUtils.h:1155
    #14 0x7fa86010b0d4 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1200
    #15 0x7fa85ce066a4 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1040:14
    #16 0x7fa85ce22660 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:517:10
    #17 0x7fa85dcd5ada in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
    #18 0x7fa85dc26789 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #19 0x7fa85dc26789 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #20 0x7fa85dc26789 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299
    #21 0x7fa8645d877a in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27
    #22 0x7fa868a8c5cb in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
    #23 0x7fa868c991c5 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4673:22
    #24 0x7fa868c9c13c in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4808:8
    #25 0x7fa868c9d584 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4900:21
    #26 0x4f6d45 in do_main /src/browser/app/nsBrowserApp.cpp:231:22
    #27 0x4f6d45 in main /src/browser/app/nsBrowserApp.cpp:304
    #28 0x7fa87c4b182f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #29 0x4265bc in _start (/home/ubuntu/firefox/firefox+0x4265bc)

0x61700022f0a5 is located 421 bytes inside of 672-byte region [0x61700022ef00,0x61700022f1a0)
freed by thread T0 here:
    #0 0x4c6fc2 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:68:3
    #1 0x7fa85cc8f530 in SnowWhiteKiller::~SnowWhiteKiller() /src/xpcom/base/nsCycleCollector.cpp:2729:25
    #2 0x7fa85cc9710f in FreeSnowWhite /src/xpcom/base/nsCycleCollector.cpp:2917:3
    #3 0x7fa85cc9710f in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:3925
    #4 0x7fa85cc966d4 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3746:9
    #5 0x7fa85cc9a3a6 in nsCycleCollector_collect(nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:4315:21
    #6 0x7fa8601b044b in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /src/dom/base/nsJSEnvironment.cpp:1505:3
    #7 0x7fa85fcf6f5b in nsDOMWindowUtils::CycleCollect(nsICycleCollectorListener*) /src/dom/base/nsDOMWindowUtils.cpp:1267:3
    #8 0x7fa85ce32cc1 in NS_InvokeByIndex /src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #9 0x7fa85e8db790 in Invoke /src/js/xpconnect/src/XPCWrappedNative.cpp:1948:12
    #10 0x7fa85e8db790 in Call /src/js/xpconnect/src/XPCWrappedNative.cpp:1267
    #11 0x7fa85e8db790 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /src/js/xpconnect/src/XPCWrappedNative.cpp:1234
    #12 0x7fa85e8e1ff3 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:929:12
    #13 0x7fa868fa765e in CallJSNative /src/js/src/vm/JSContext-inl.h:291:15
    #14 0x7fa868fa765e in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #15 0x7fa868f90f55 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12
    #16 0x7fa868f90f55 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096
    #17 0x7fa868f73094 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #18 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #19 0x7fa868fa81c3 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:541:10
    #20 0x7fa869be07f5 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2978:12
    #21 0x7fa85e7f64f0 in xpc::FunctionForwarder(JSContext*, unsigned int, JS::Value*) /src/js/xpconnect/src/ExportHelpers.cpp:315:18
    #22 0x7fa868fa765e in CallJSNative /src/js/src/vm/JSContext-inl.h:291:15
    #23 0x7fa868fa765e in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #24 0x7fa868f90f55 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12
    #25 0x7fa868f90f55 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096
    #26 0x7fa868f73094 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #27 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #28 0x7fa868fa81c3 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:541:10
    #29 0x7fa869be316f in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:3037:12
    #30 0x7fa861f50952 in mozilla::dom::BlobCallback::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Blob*, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/HTMLCanvasElementBinding.cpp:83:8
    #31 0x7fa8626d082a in Call /src/obj-firefox/dist/include/mozilla/dom/HTMLCanvasElementBinding.h:179:12
    #32 0x7fa8626d082a in mozilla::dom::CanvasRenderingContextHelper::ToBlob(JSContext*, nsIGlobalObject*, mozilla::dom::BlobCallback&, nsTSubstring<char16_t> const&, JS::Handle<JS::Value>, bool, mozilla::ErrorResult&)::EncodeCallback::ReceiveBlob(already_AddRefed<mozilla::dom::Blob>) /src/dom/canvas/CanvasRenderingContextHelper.cpp:48
    #33 0x7fa85ff32e59 in mozilla::dom::EncodingCompleteEvent::Run() /src/dom/base/ImageEncoder.cpp:109:37
    #34 0x7fa85ce066a4 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1040:14
    #35 0x7fa85ce22660 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:517:10
    #36 0x7fa85dcd5ada in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
    #37 0x7fa85dc26789 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:326:10
    #38 0x7fa85dc26789 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:319
    #39 0x7fa85dc26789 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299

previously allocated by thread T0 here:
    #0 0x4c7303 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x4f7dcd in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:70:17
    #2 0x7fa864311ee2 in operator new /src/obj-firefox/dist/include/mozilla/mozalloc.h:159:12
    #3 0x7fa864311ee2 in mozilla::dom::XMLHttpRequest::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::MozXMLHttpRequestParameters const&, mozilla::ErrorResult&) /src/dom/xhr/XMLHttpRequest.cpp:28
    #4 0x7fa8619de235 in mozilla::dom::XMLHttpRequestBinding::_constructor(JSContext*, unsigned int, JS::Value*) /src/obj-firefox/dom/bindings/XMLHttpRequestBinding.cpp:2502:64
    #5 0x7fa868fa8a7e in CallJSNative /src/js/src/vm/JSContext-inl.h:291:15
    #6 0x7fa868fa8a7e in CallJSNativeConstructor /src/js/src/vm/JSContext-inl.h:324
    #7 0x7fa868fa8a7e in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /src/js/src/vm/Interpreter.cpp:580
    #8 0x7fa868f90eed in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3088:18
    #9 0x7fa868f73094 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #10 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #11 0x7fa868fa81c3 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:541:10
    #12 0x7fa8690b8e46 in js::PromiseObject::create(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, bool) /src/js/src/builtin/Promise.cpp:1667:19
    #13 0x7fa8691b5ae7 in PromiseConstructor(JSContext*, unsigned int, JS::Value*) /src/js/src/builtin/Promise.cpp:1595:30
    #14 0x7fa868fa899d in CallJSNative /src/js/src/vm/JSContext-inl.h:291:15
    #15 0x7fa868fa899d in CallJSNativeConstructor /src/js/src/vm/JSContext-inl.h:324
    #16 0x7fa868fa899d in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /src/js/src/vm/Interpreter.cpp:568
    #17 0x7fa868f90eed in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3088:18
    #18 0x7fa868f73094 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #19 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #20 0x7fa868fa81c3 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:541:10
    #21 0x7fa869a748c6 in js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /src/js/src/jit/VMFunctions.cpp:954:12
    #22 0x7fa80aef1236  (<unknown module>)
    #23 0x7fa80aef04e7  (<unknown module>)
    #24 0x7fa869655142 in EnterJit /src/js/src/jit/Jit.cpp:99:9
    #25 0x7fa869655142 in js::jit::MaybeEnterJit(JSContext*, js::RunState&) /src/js/src/jit/Jit.cpp:163
    #26 0x7fa868f72db6 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:408:34
    #27 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15
    #28 0x7fa868fa81c3 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:541:10
    #29 0x7fa86a2eb2d3 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/SelfHosting.cpp:1735:12
    #30 0x7fa869ee286e in AsyncFunctionResume(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>, ResumeKind, JS::Handle<JS::Value>) /src/js/src/vm/AsyncFunction.cpp:190:10
    #31 0x7fa869ee15c6 in AsyncFunctionStart /src/js/src/vm/AsyncFunction.cpp:203:12
    #32 0x7fa869ee15c6 in WrappedAsyncFunction(JSContext*, unsigned int, JS::Value*) /src/js/src/vm/AsyncFunction.cpp:89
    #33 0x7fa868fa765e in CallJSNative /src/js/src/vm/JSContext-inl.h:291:15
    #34 0x7fa868fa765e in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:473
    #35 0x7fa868f90f55 in CallFromStack /src/js/src/vm/Interpreter.cpp:528:12
    #36 0x7fa868f90f55 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3096
    #37 0x7fa868f73094 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:423:12
    #38 0x7fa868fa7457 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:15

Comment 1

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Testcase requires fuzzPriv extension.

The best way I've managed to make this repro is to open it on browser launch. It can take a few attempts but usually <5.

Comment 2

[Tyson Smith [:tsmith]]

Attachment: prefs.js

Here is the pref.js in case it is needed.

Comment 3

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

Hmm, is that really the right testcase. It doesn't use XHR, yet stack trace shows an XHR is created. 

Trying to reproduce.

Comment 4

[Tyson Smith [:tsmith]]

(In reply to Olli Pettay [:smaug] from comment #3)
> Hmm, is that really the right testcase.

Yes. 

I would assume this is a new issue that popped up just before the weekend because I am seeing all sorts variations of this issue.

Comment 5

[Tyson Smith [:tsmith]]

Attachment: minidump.log

Here is a crash in a debug build that appeared at the same time and I am also seeing frequently.

Comment 6

[Andrew McCreight [:mccr8]]

Related to bug 792808 maybe? That's a large XHR change that landed on 2/16.

Comment 7

[Thomas Wisniewski]

I don't why it would be related to that patchset, but there's always a chance. Where would I get a nightly-compatible fuzzpriv version so I can try to find a regression range?

Comment 8

[Tyson Smith [:tsmith]]

(In reply to Thomas Wisniewski from comment #7)
> Where would I get a nightly-compatible fuzzpriv version so I can try to find a regression range?

All versions of fuzzPriv should still be compatible with Nightly, it is not compatible with beta and release builds. https://www.squarefree.com/extensions/domFuzzLite3.xpi

Comment 9

[Thomas Wisniewski]

Attachment: candidate_fix.diff

Ah, I see now how the patchset from bug 792808 could cause this issue, and I believe this patch would fix it.

Baku, do you agree? It seems to me that what is happening here is that an XHR can die, but a parse-end event can still then chance to fire before the parse-end listener dies (and it will then try to access the now-dead XHR). This patch tells it not to try to access the dead XHR anymore.

Comment 10

[Tyson Smith [:tsmith]]

I have tested with and without the patch and the issue no longer reproduces with the patch applied.

Comment 11

[Andrea Marchesini [:baku]]

Comment on attachment 8952511 [details] [diff] [review]
candidate_fix.diff

Review of attachment 8952511 [details] [diff] [review]:
-----------------------------------------------------------------

Don't add a friend class, but a Nullify/Reset/Forget() method that does mXHR = nullptr;

Comment 12

[Thomas Wisniewski]

Attachment: fix.diff

Alright, I've addressed the review comment in this fix. Carrying over r+.

tsmith, I don't suspect this patch will cause any try-failures, so should we just land it as-is?

Comment 13

[Tyson Smith [:tsmith]]

Sure that's fine with me.

Comment 14

[Thomas Wisniewski]

Alright then, requesting check-in.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Please attach a patch that has the necessary commit information included.

Comment 16

[Thomas Wisniewski]

Attachment: 1439022.diff

Here's an updated patch with commit info.

Comment 17

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/462c61a4d44421763825716a6d9958670154fa8a

Comment 18

[Ryan VanderMeulen [:RyanVM]]

FWIW, I think we could have used a slightly more descriptive commit message here since this only affected trunk anyway ;)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/462c61a4d444