Closed Bug 1439022 Opened 4 years ago Closed 4 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed

People

(Reporter: tsmith, Assigned: wisniewskit)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, sec-high, testcase)

Attachments

(4 files, 2 obsolete files)

testcase.html
582 bytes, text/html
Details
prefs.js
10.70 KB, application/x-javascript
Details
minidump.log
4.95 KB, text/plain
Details
1439022.diff
1.65 KB, patch
Details | Diff | Splinter Review 
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
Attached file 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.
Flags: in-testsuite?
Keywords: testcase-wanted → testcase
Attached file prefs.js — 
Here is the pref.js in case it is needed.
Hmm, is that really the right testcase. It doesn't use XHR, yet stack trace shows an XHR is created. 

Trying to reproduce.
(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.
Attached file minidump.log — 
Here is a crash in a debug build that appeared at the same time and I am also seeing frequently.
Attachment #8952385 - Attachment mime type: text/x-log → text/plain
Related to bug 792808 maybe? That's a large XHR change that landed on 2/16.
Flags: needinfo?(wisniewskit)
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?
Flags: needinfo?(wisniewskit)
(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
Attached patch candidate_fix.diff (obsolete) — — Splinter Review 
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.
Attachment #8952511 - Flags: review?(amarchesini)
I have tested with and without the patch and the issue no longer reproduces with the patch applied.
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;
Attachment #8952511 - Flags: review?(amarchesini) → review+
Attached patch fix.diff (obsolete) — — Splinter Review 
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?
Attachment #8952511 - Attachment is obsolete: true
Flags: needinfo?(twsmith)
Sure that's fine with me.
Flags: needinfo?(twsmith)
Alright then, requesting check-in.
Keywords: checkin-needed
Please attach a patch that has the necessary commit information included.
Assignee: nobody → wisniewskit
Blocks: 792808
status-firefox58: --- → unaffected
status-firefox59: --- → unaffected
status-firefox-esr52: --- → unaffected
Flags: needinfo?(wisniewskit)
Keywords: checkin-needed
Keywords: sec-high
Attached patch 1439022.diff — — Splinter Review 
Here's an updated patch with commit info.
Attachment #8952762 - Attachment is obsolete: true
Flags: needinfo?(wisniewskit)
Keywords: checkin-needed
FWIW, I think we could have used a slightly more descriptive commit message here since this only affected trunk anyway ;)
Keywords: checkin-needed, crash
https://hg.mozilla.org/mozilla-central/rev/462c61a4d444
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox60: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Group: dom-core-security → core-security-release
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.