1533913 - crash near null in [@ nsINode::Length]

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

==28359==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7fad22ecde9a bp 0x7fffa2f86b50 sp 0x7fffa2f86b40 T0)
==28359==The signal is caused by a READ memory access.
==28359==Hint: address points to the zero page.
    #0 0x7fad22ecde99 in get src/obj-firefox/dist/include/mozilla/RefPtr.h:267:27
    #1 0x7fad22ecde99 in operator-> src/obj-firefox/dist/include/mozilla/RefPtr.h:297
    #2 0x7fad22ecde99 in NodeType src/dom/base/nsINode.h:638
    #3 0x7fad22ecde99 in nsINode::Length() const src/dom/base/nsINode.cpp:2464
    #4 0x7fad2903ae45 in mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent> >::SetToEndOf(nsINode const*) src/obj-firefox/dist/include/mozilla/EditorDOMPoint.h:334:38
    #5 0x7fad290926bb in InvalidateOffset src/obj-firefox/dist/include/mozilla/EditorDOMPoint.h:773:14
    #6 0x7fad290926bb in ~AutoEditorDOMPointOffsetInvalidator src/obj-firefox/dist/include/mozilla/EditorDOMPoint.h:760
    #7 0x7fad290926bb in mozilla::HTMLEditRules::ApplyBlockStyle(nsTArray<mozilla::OwningNonNull<nsINode> >&, nsAtom&) src/editor/libeditor/HTMLEditRules.cpp:8667
    #8 0x7fad29043a81 in mozilla::HTMLEditRules::MakeBasicBlock(nsAtom&) src/editor/libeditor/HTMLEditRules.cpp:4473:10
    #9 0x7fad29021d86 in mozilla::HTMLEditRules::WillMakeBasicBlock(nsTSubstring<char16_t> const&, bool*, bool*) src/editor/libeditor/HTMLEditRules.cpp:4302:8
    #10 0x7fad29007975 in mozilla::HTMLEditRules::WillDoAction(mozilla::EditSubActionInfo&, bool*, bool*) src/editor/libeditor/HTMLEditRules.cpp:708:14
    #11 0x7fad291017f1 in mozilla::HTMLEditor::InsertBasicBlockWithTransaction(nsAtom&) src/editor/libeditor/HTMLEditor.cpp:2294:24
    #12 0x7fad291005e8 in mozilla::HTMLEditor::SetParagraphFormat(nsTSubstring<char16_t> const&) src/editor/libeditor/HTMLEditor.cpp:1825:10
    #13 0x7fad2912e444 in mozilla::MultiStateCommandBase::DoCommandParams(char const*, nsICommandParams*, nsISupports*) src/editor/libeditor/HTMLEditorCommands.cpp:590:10
    #14 0x7fad26661200 in nsControllerCommandTable::DoCommandParams(char const*, nsICommandParams*, nsISupports*) src/dom/commandhandler/nsControllerCommandTable.cpp:155:26
    #15 0x7fad26658e18 in DoCommandWithParams src/dom/commandhandler/nsBaseCommandController.cpp:138:25
    #16 0x7fad26658e18 in non-virtual thunk to nsBaseCommandController::DoCommandWithParams(char const*, nsICommandParams*) src/dom/commandhandler/nsBaseCommandController.cpp
    #17 0x7fad2665cf68 in nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) src/dom/commandhandler/nsCommandManager.cpp:197:29
    #18 0x7fad26d0014d in nsHTMLDocument::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) src/dom/html/nsHTMLDocument.cpp:2562:18
    #19 0x7fad259b9f1e in mozilla::dom::HTMLDocument_Binding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/HTMLDocumentBinding.cpp:619:21
    #20 0x7fad25f5efd1 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:3144:13
    #21 0x7fad2d5ca657 in CallJSNative src/js/src/vm/Interpreter.cpp:440:13
    #22 0x7fad2d5ca657 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:532
    #23 0x7fad2d5b1d50 in CallFromStack src/js/src/vm/Interpreter.cpp:591:10
    #24 0x7fad2d5b1d50 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3056
    #25 0x7fad2d594678 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:420:10
    #26 0x7fad2d5cafc6 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:560:13
    #27 0x7fad2d5ccc12 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:603:8
    #28 0x7fad2e1cfc09 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2623:10
    #29 0x7fad255657e9 in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:266:37
    #30 0x7fad26811d29 in void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:363:12
    #31 0x7fad2680efb9 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) src/dom/events/JSEventHandler.cpp:205:12
    #32 0x7fad267c159a in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1042:51
    #33 0x7fad267c3b73 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1237:17
    #34 0x7fad267a3cf0 in HandleEvent src/obj-firefox/dist/include/mozilla/EventListenerManager.h:350:5
    #35 0x7fad267a3cf0 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:351
    #36 0x7fad267a1f18 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:553:16
    #37 0x7fad267a8b63 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1048:11
    #38 0x7fad29646f67 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1102:7
    #39 0x7fad2c48a58c in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:6560:21
    #40 0x7fad2c4896b8 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:6361:7
    #41 0x7fad2c48f227 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
    #42 0x7fad212fd645 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1313:3
    #43 0x7fad212fc22c in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:872:14
    #44 0x7fad212f6861 in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:710:9
    #45 0x7fad212fa480 in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:598:5
    #46 0x7fad212fbd54 in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp
    #47 0x7fad1ead39e7 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:568:22
    #48 0x7fad22b16caa in DoUnblockOnload src/dom/base/Document.cpp:7705:18
    #49 0x7fad22b16caa in mozilla::dom::Document::UnblockOnload(bool) src/dom/base/Document.cpp:7637
    #50 0x7fad22b1570f in mozilla::dom::Document::DispatchContentLoadedEvents() src/dom/base/Document.cpp:4787:3
    #51 0x7fad22c18cab in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1122:12
    #52 0x7fad22c18cab in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1128
    #53 0x7fad22c18cab in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1174
    #54 0x7fad1e815ac5 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:295:32
    #55 0x7fad1e855351 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1179:14
    #56 0x7fad1e85d75d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:482:10
    #57 0x7fad1faf6fff in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
    #58 0x7fad1f9cecce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #59 0x7fad1f9cecce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #60 0x7fad1f9cecce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #61 0x7fad28db2123 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #62 0x7fad2d2eb48e in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:911:20
    #63 0x7fad1f9cecce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #64 0x7fad1f9cecce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #65 0x7fad1f9cecce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #66 0x7fad2d2ea5e3 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:749:34
    #67 0x5580ce7f8874 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:49:28
    #68 0x5580ce7f8874 in main src/browser/app/nsBrowserApp.cpp:265

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

It seems that aNodeArray of ApplyBlockStyle() may have orphan nodes?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, or, can become orphan nodes because of the DOM tree changes?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, similar case could be with cmd_align for list elements, but I've not succeeded to create such testcase...

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1533913 - Make HTMLEditRules::ApplyBlockStyle() stop handling it if target node is moved from expected container unexpectedly

HTMLEditRules::ApplyBlockStyle() stores curBlock and newBlock during its
loop to keep handling from deeper child to ancestor, and may do two things for
a curNode. If curBlock and/or newBlock is moved from expected container
when it sets one of or both of them, this patch check whether mutation event
listeners change the DOM tree. Additionally, this patch also checks whether
`curNode' is moved by mutation event listener at first step of two jobs for it.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bfc4c1e72488
Make HTMLEditRules::ApplyBlockStyle() stop handling it if target node is moved from expected container unexpectedly r=m_kato

Comment 6

[Noemi Erli[:noemi_erli]]

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

Comment 7

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9067744 [details]
Bug 1533913 - Make HTMLEditRules::ApplyBlockStyle() stop handling it if target node is moved from expected container unexpectedly

Beta/Release Uplift Approval Request

• User impact if declined: User may meet this crash if attacker tries to crash our user's tab.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• 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): This patch just makes related code not touch outside of current editing host (siblings/parents of current editing host which has focus and contenteditable="true").
• String changes made/needed: none

Comment 9

[Julien Cristau [:jcristau]]

It doesn't look like this is hitting in the wild, so I think the fix can ride the trains.