1865729 - Assertion failure: movedContentRange.StartRef().EqualsOrIsBefore(pointToInsert), at /editor/libeditor/HTMLEditorDeleteHandler.cpp:5475

Status: VERIFIED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev c3021f5ece18 (built with: --enable-debug --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build c3021f5ece18 --debug --fuzzing  -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Assertion failure: movedContentRange.StartRef().EqualsOrIsBefore(pointToInsert), at /editor/libeditor/HTMLEditorDeleteHandler.cpp:5475

    ==462364==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f1a17e1a23a bp 0x7ffcc12c51e0 sp 0x7ffcc12c4b10 T462364)
    ==462364==The signal is caused by a WRITE memory access.
    ==462364==Hint: address points to the zero page.
        #0 0x7f1a17e1a23a in mozilla::HTMLEditor::AutoMoveOneLineHandler::Run(mozilla::HTMLEditor&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:5475:7
        #1 0x7f1a17e95ead in mozilla::WhiteSpaceVisibilityKeeper::MergeFirstLineOfRightBlockElementIntoDescendantLeftBlockElement(mozilla::HTMLEditor&, mozilla::dom::Element&, mozilla::dom::Element&, mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent>> const&, mozilla::Maybe<nsAtom*> const&, mozilla::dom::HTMLBRElement const*, mozilla::dom::Element const&) /editor/libeditor/WSRunObject.cpp:319:35
        #2 0x7f1a17e09eaf in mozilla::HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::AutoInclusiveAncestorBlockElementsJoiner::Run(mozilla::HTMLEditor&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:4998:14
        #3 0x7f1a17e08d9c in mozilla::HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::HandleDeleteAtOtherBlockBoundary(mozilla::HTMLEditor&, short, short, mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent>> const&, mozilla::AutoRangeArray&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:2722:18
        #4 0x7f1a17e05047 in mozilla::HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::Run(mozilla::HTMLEditor&, short, short, mozilla::EditorDOMPointBase<nsCOMPtr<nsINode>, nsCOMPtr<nsIContent>> const&, mozilla::AutoRangeArray&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:534:15
        #5 0x7f1a17dff383 in mozilla::HTMLEditor::AutoDeleteRangesHandler::HandleDeleteAroundCollapsedRanges(mozilla::HTMLEditor&, short, short, mozilla::AutoRangeArray&, mozilla::WSRunScanner const&, mozilla::WSScanResult const&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:1952:56
        #6 0x7f1a17dfb17a in mozilla::HTMLEditor::AutoDeleteRangesHandler::Run(mozilla::HTMLEditor&, short, short, mozilla::AutoRangeArray&, mozilla::dom::Element const&) /editor/libeditor/HTMLEditorDeleteHandler.cpp:1698:11
        #7 0x7f1a17dfa0ff in mozilla::HTMLEditor::HandleDeleteSelection(short, short) /editor/libeditor/HTMLEditorDeleteHandler.cpp:1165:61
        #8 0x7f1a17d23d1c in mozilla::EditorBase::DeleteSelectionAsSubAction(short, short) /editor/libeditor/EditorBase.cpp:4442:9
        #9 0x7f1a17d1e2e0 in mozilla::EditorBase::DeleteSelectionAsAction(short, short, nsIPrincipal*) /editor/libeditor/EditorBase.cpp:4405:8
        #10 0x7f1a17d3dce8 in mozilla::DeleteCommand::DoCommand(mozilla::Command, mozilla::EditorBase&, nsIPrincipal*) const /editor/libeditor/EditorCommands.cpp:623:29
        #11 0x7f1a142b16d7 in mozilla::dom::Document::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) /dom/base/Document.cpp:5505:37
        #12 0x7f1a1556fc11 in mozilla::dom::Document_Binding::execCommand(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/./DocumentBinding.cpp:4007:36
        #13 0x7f1a158bf5d8 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /dom/bindings/BindingUtils.cpp:3330:13
        #14 0x7f1a1a099be4 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:472:13
        #15 0x7f1a1a0994fd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:566:12
        #16 0x7f1a1a0a9ac8 in CallFromStack /js/src/vm/Interpreter.cpp:638:10
        #17 0x7f1a1a0a9ac8 in js::Interpret(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:3053:16
        #18 0x7f1a1a098a52 in js::RunScript(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:444:13
        #19 0x7f1a1a099519 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:598:13
        #20 0x7f1a1a09a9bd in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /js/src/vm/Interpreter.cpp:665:8
        #21 0x7f1a1a181ca4 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /js/src/vm/CallAndConstruct.cpp:119:10
        #22 0x7f1a155d1e9c in mozilla::dom::EventListener::HandleEvent(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./EventListenerBinding.cpp:62:8
        #23 0x7f1a15f301e6 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventListenerBinding.h:65:12
        #24 0x7f1a15f2fda2 in mozilla::EventListenerManager::HandleEventSingleListener(mozilla::EventListenerManager::Listener*, nsAtom*, mozilla::WidgetEvent*, mozilla::dom::Event*, mozilla::dom::EventTarget*, bool) /dom/events/EventListenerManager.cpp:1342:43
        #25 0x7f1a15f30ee4 in mozilla::EventListenerManager::HandleEventWithListenerArray(mozilla::EventListenerManager::ListenerArray*, nsAtom*, mozilla::EventMessage, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, bool) /dom/events/EventListenerManager.cpp:1663:12
        #26 0x7f1a15f30759 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /dom/events/EventListenerManager.cpp:1560:35
        #27 0x7f1a15f23d4f in HandleEvent /builds/worker/workspace/obj-build/dist/include/mozilla/EventListenerManager.h:465:5
        #28 0x7f1a15f23d4f in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /dom/events/EventDispatcher.cpp:364:17
        #29 0x7f1a15f232cb in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /dom/events/EventDispatcher.cpp:611:18
        #30 0x7f1a15f25d06 in mozilla::EventDispatcher::Dispatch(mozilla::dom::EventTarget*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /dom/events/EventDispatcher.cpp:1232:11
        #31 0x7f1a1807b3b2 in nsDocumentViewer::LoadComplete(nsresult) /layout/base/nsDocumentViewer.cpp:1077:7
        #32 0x7f1a196841d2 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /docshell/base/nsDocShell.cpp:6348:20
        #33 0x7f1a196835db in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /docshell/base/nsDocShell.cpp:5740:7
        #34 0x7f1a196852a6 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /docshell/base/nsDocShell.cpp
        #35 0x7f1a136daa09 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /uriloader/base/nsDocLoader.cpp:1372:3
        #36 0x7f1a136d9f82 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /uriloader/base/nsDocLoader.cpp:978:14
        #37 0x7f1a136d813b in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) /uriloader/base/nsDocLoader.cpp:795:9
        #38 0x7f1a136d93e1 in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /uriloader/base/nsDocLoader.cpp:678:5
        #39 0x7f1a196bae3f in nsDocShell::OnStopRequest(nsIRequest*, nsresult) /docshell/base/nsDocShell.cpp:13813:23
        #40 0x7f1a128fc2ff in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) /netwerk/base/nsLoadGroup.cpp:631:22
        #41 0x7f1a128fd840 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /netwerk/base/nsLoadGroup.cpp:535:10
        #42 0x7f1a142dec3c in mozilla::dom::Document::DoUnblockOnload() /dom/base/Document.cpp:11680:18
        #43 0x7f1a142c4ad6 in mozilla::dom::Document::DispatchContentLoadedEvents() /dom/base/Document.cpp:8133:3
        #44 0x7f1a14377f49 in operator()<> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1164:18
        #45 0x7f1a14377f49 in __invoke_impl<void, (lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:60:14
        #46 0x7f1a14377f49 in __invoke<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:95:14
        #47 0x7f1a14377f49 in __apply_impl<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1678:14
        #48 0x7f1a14377f49 in apply<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1687:14
        #49 0x7f1a14377f49 in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1162:12
        #50 0x7f1a14377f49 in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1213:13
        #51 0x7f1a126bb8e7 in mozilla::RunnableTask::Run() /xpcom/threads/TaskController.cpp:549:16
        #52 0x7f1a126b34b3 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:876:26
        #53 0x7f1a126b1cf7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /xpcom/threads/TaskController.cpp:699:15
        #54 0x7f1a126b2155 in mozilla::TaskController::ProcessPendingMTTask(bool) /xpcom/threads/TaskController.cpp:485:36
        #55 0x7f1a126bf5f6 in operator() /xpcom/threads/TaskController.cpp:211:37
        #56 0x7f1a126bf5f6 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /xpcom/threads/nsThreadUtils.h:548:5
        #57 0x7f1a126d6152 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1198:16
        #58 0x7f1a126dd23d in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:480:10
        #59 0x7f1a13398355 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #60 0x7f1a132b2281 in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #61 0x7f1a132b2281 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #62 0x7f1a17c032d8 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #63 0x7f1a19e5ab9b in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:721:20
        #64 0x7f1a13399236 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:235:9
        #65 0x7f1a132b2281 in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #66 0x7f1a132b2281 in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #67 0x7f1a19e5a402 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:656:34
        #68 0x55c407890276 in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #69 0x55c407890276 in main /browser/app/nsBrowserApp.cpp:375:18
        #70 0x7f1a27f91d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #71 0x7f1a27f91e3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #72 0x55c407865fa8 in _start (/home/jkratzer/builds/m-c-20231119091854-fuzzing-debug/firefox-bin+0x58fa8) (BuildId: 7a796fcd43097257b4ee5a0ae21d134c2f0df8b9)
    
    UndefinedBehaviorSanitizer can not provide additional info.
    SUMMARY: UndefinedBehaviorSanitizer: SEGV /editor/libeditor/HTMLEditorDeleteHandler.cpp:5475:7 in mozilla::HTMLEditor::AutoMoveOneLineHandler::Run(mozilla::HTMLEditor&, mozilla::dom::Element const&)
    ==462364==ABORTING

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20231120173116-e39cc33d2356.
The bug appears to have been introduced in the following build range:

Start: cc2d7a60e797ccda49d1a49206bcded220dd6c4b (20231101012948)
End: b73ef4c8979fb0702de9f4bf2fdf986b1d0fd487 (20231101040821)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cc2d7a60e797ccda49d1a49206bcded220dd6c4b&tochange=b73ef4c8979fb0702de9f4bf2fdf986b1d0fd487

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Hmm, in this case, AutoMoveOneLineHandler tries to move a line into a line in it. Therefore, the destination may become in an orphan node so that the DOM point comparison fails. (Moving all content of <body> to end of the <body><svg><a display="table-header-group">.)

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1865729 - part 0: Add logging code of `AutoMoveOneLineHandler` r=m_kato!

The class does complicated things and has been reported some assertion failures.
For making the debug faster, let's add the logging code.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1865729 - part 1: Make `AutoMoveOneLineHandler` make `AutoRangeArray` work with `BlockInlineCheck::UseComputedDisplayOutsideStyle` r=m_kato!

AutoMoveOneLineHandler uses AutoRangeArray API for block level edit
sub-actions. Therefore, the source line is computed with
BlockInlineCheck::UseHTMLDefaultStyle. However, the deletion handler works
with BlockInlineCheck::UseComputedDisplayOutsideStyle. Therefore,
AutoMoveOneLineHandler may try to move different range. In the reported
test case, it tries to move all content under the <body> into the
<a display="table-header-group"> which is contained in the range. Therefore,
the movedContentRange check fails after the destination becomes into an
orphan node which was removed to move.

This patch renames the API and adds a BlockInlineCheck parameter to work
with both ways and makes AutoMoveOneLineHandler specify
BlockInlineCheck::UseComputedDisplayOutsideStyle as same as the other delete
handlers.

Finally, same thing may happen in
HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal. This patch makes it
check whether the top-level edit sub-action is a block level one or not and
consider BlockInlineCheck with the result.

Depends on D194180

Comment 6

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

Based on comment #2, this bug contains a bisection range found by bugmon. However, the Regressed by field is still not filled.

:masayuki, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit BugBot documentation.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

This is directly regressed by bug 1858794, but the root cause is a bug of the patch for bug 1851951. Therefore, this blocks bug 1858071 which will enable the fix of bug 1851951 in the all channels.

Comment 8

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

Set release status flags based on info from the regressing bug 1858794

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5608a7806b8
part 0: Add logging code of `AutoMoveOneLineHandler` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/edf9292c0bc9
part 1: Make `AutoMoveOneLineHandler` make `AutoRangeArray` work with `BlockInlineCheck::UseComputedDisplayOutsideStyle` r=m_kato

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43356 for changes under testing/web-platform/tests

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/e5608a7806b8
https://hg.mozilla.org/mozilla-central/rev/edf9292c0bc9

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

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

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox121 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20231127092818-edf9292c0bc9.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

This is only an assertion failure which does not cause a real crash, and there is no web-compat issue report caused by this. Additionally, the patch is a little bit risky. Therefore, I think that it should just ride the train.