1619322 - AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/base/nsINode.h:1704:12 in GetBoolFlag

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 51efc4b931f7.

==7489==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000001c (pc 0x7f79647b7097 bp 0x7ffd015a2690 sp 0x7ffd015a2580 T0)
==7489==The signal is caused by a READ memory access.
==7489==Hint: address points to the zero page.
    #0 0x7f79647b7096 in GetBoolFlag /builds/worker/workspace/build/src/dom/base/nsINode.h:1704:12
    #1 0x7f79647b7096 in IsElement /builds/worker/workspace/build/src/dom/base/nsINode.h:494:35
    #2 0x7f79647b7096 in mozilla::dom::Document::RemoveChildNode(nsIContent*, bool) /builds/worker/workspace/build/src/dom/base/Document.cpp:6493:13
    #3 0x7f7964aa5a0f in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/nsINode.cpp:2510:5
    #4 0x7f7964aa2082 in nsINode::ReplaceWith(mozilla::dom::Sequence<mozilla::dom::OwningNodeOrString> const&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/dom/base/nsINode.h
    #5 0x7f7965fe690d in mozilla::dom::Element_Binding::replaceWith(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/ElementBinding.cpp:5348:24
    #6 0x7f79663ea918 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3171:13
    #7 0x7f796c913993 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:477:13
    #8 0x7f796c913993 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:569:12
    #9 0x7f796c91578a in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:632:10
    #10 0x7f796c8fa1f9 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:636:10
    #11 0x7f796c8fa1f9 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3046:16
    #12 0x7f796c8dd731 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:449:10
    #13 0x7f796c913a75 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:604:13
    #14 0x7f796c91578a in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:632:10
    #15 0x7f796c915a66 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:649:8
    #16 0x7f796cab1c82 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2797:10
    #17 0x7f79660b217d in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:41:8
    #18 0x7f796498a564 in void mozilla::dom::Function::Call<nsCOMPtr<nsIGlobalObject> >(nsCOMPtr<nsIGlobalObject> const&, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:73:12
    #19 0x7f796498a1d8 in mozilla::dom::CallbackTimeoutHandler::Call(char const*) /builds/worker/workspace/build/src/dom/base/TimeoutHandler.cpp:167:29
    #20 0x7f79645bc26f in nsGlobalWindowInner::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) /builds/worker/workspace/build/src/dom/base/nsGlobalWindowInner.cpp:5918:38
    #21 0x7f7964985949 in mozilla::dom::TimeoutManager::RunTimeout(mozilla::TimeStamp const&, mozilla::TimeStamp const&, bool) /builds/worker/workspace/build/src/dom/base/TimeoutManager.cpp:892:44
    #22 0x7f796498466f in mozilla::dom::TimeoutExecutor::MaybeExecute() /builds/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp:179:11
    #23 0x7f7964987ef6 in Notify /builds/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp:246:5
    #24 0x7f7964987ef6 in non-virtual thunk to mozilla::dom::TimeoutExecutor::Notify(nsITimer*) /builds/worker/workspace/build/src/dom/base/TimeoutExecutor.cpp
    #25 0x7f79608232bc in nsTimerImpl::Fire(int) /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:565:39
    #26 0x7f7960822a8c in nsTimerEvent::Run() /builds/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:250:11
    #27 0x7f796085ba4e in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() /builds/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:252:22
    #28 0x7f7960851e4f in mozilla::ThrottledEventQueue::Inner::Executor::Run() /builds/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:80:15
    #29 0x7f7960801dad in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:282:20
    #30 0x7f7960836328 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1220:14
    #31 0x7f796084118c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:481:10
    #32 0x7f7961aa7de4 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:109:5
    #33 0x7f796199a7c7 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #34 0x7f796199a7c7 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #35 0x7f796199a7c7 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #36 0x7f7968b31738 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
    #37 0x7f796c6adb76 in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:926:20
    #38 0x7f796199a7c7 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #39 0x7f796199a7c7 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #40 0x7f796199a7c7 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #41 0x7f796c6ad238 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:761:34
    #42 0x559231b23f83 in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #43 0x559231b23f83 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:303:18
    #44 0x7f7983493b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/dom/base/nsINode.h:1704:12 in GetBoolFlag

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: prefs.js

Comment 2

[Jason Kratzer [:jkratzer]]

BugMon: Verified bug as reproducible on 51efc4b931f7

Comment 3

[Jason Kratzer [:jkratzer]]

BugMon: Failed to bisect testcase (Start build crashes!)
> Start: 8cdde0ff59a15f59736e810524a649a4c8a5a7ef (20190304093620)
> End: 51efc4b931f748899be0fa3c9603fc4e07b668b6 (20200302094818)

Comment 4

[Henri Sivonen (:hsivonen)]

Assigning to self for debugging. Might unassign after.

Comment 5

[Henri Sivonen (:hsivonen)]

We blow away the document via document.writeln at
https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/dom/base/nsINode.cpp#2294

Yet, the mutation guard check evaluates to false afterwards:
https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/dom/base/nsINode.cpp#2310
...so we don't re-run EnsurePreInsertionValidity2.

Since the fragment is empty, we pretty much skip a bunch of code.

And then we encounter an assertion at:
https://searchfox.org/mozilla-central/rev/91f6c02fcf4c16f78fdc4417f61f192688294066/dom/base/nsINode.cpp#2508

needinfoing bz, who wrote the mutation guard check that, to me, looks like should have caught this but didn't.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oof. We call nsMutationGuard::DidMutate in nsINode::RemoveChildNode, but Document::Open does not call RemoveChildNode, because it's trying to remove kids without firing mutation events, etc. We need to call DidMutate in Document::DisconnectNodeTree. Lemme try that.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I am pretty sure this is always a null deref, by the way....

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1619322. Make sure to call DidMutate() when clearing the document on document.open.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7958fc833bed
Make sure to call DidMutate() when clearing the document on document.open. r=hsivonen

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7958fc833bed