Bug 208628 - Nullptr crash in CompositeEditCommand::moveParagraphWithClones when indenting non-enclosed elements.
Summary: Nullptr crash in CompositeEditCommand::moveParagraphWithClones when indenting...
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2020-03-05 00:29 PST by Jack
Modified: 2020-03-10 16:41 PDT (History)
7 users (show)

See Also:

Patch (4.16 KB, patch)
2020-03-05 01:26 PST, Jack
no flags Details | Formatted Diff | Diff
Patch (4.17 KB, patch)
2020-03-05 01:32 PST, Jack
no flags Details | Formatted Diff | Diff
Patch (4.33 KB, patch)
2020-03-05 02:48 PST, Jack
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jack 2020-03-05 00:29:19 PST

==42480==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000018 (pc 0x7fff4191f32f bp 0x7ffeef05af90 sp 0x7ffeef05af20 T0)
==42480==The signal is caused by a READ memory access.
==42480==Hint: address points to the zero page.
    #0 0x7fff4191f32e in WebCore::CompositeEditCommand::cloneParagraphUnderNewElement(WebCore::Position const&, WebCore::Position const&, WebCore::Node*, WebCore::Element*) (WebCore:x86_64+0xf0332e)
    #1 0x7fff4191f9f7 in WebCore::CompositeEditCommand::moveParagraphWithClones(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&, WebCore::Element*, WebCore::Node*) (WebCore:x86_64+0xf039f7)
    #2 0x7fff4194fa9d in WebCore::IndentOutdentCommand::indentIntoBlockquote(WebCore::Position const&, WebCore::Position const&, WTF::RefPtr<WebCore::Element, WTF::DumbPtrTraits<WebCore::Element> >&) (WebCore:x86_64+0xf33a9d)
    #3 0x7fff4190c087 in WebCore::ApplyBlockElementCommand::formatSelection(WebCore::VisiblePosition const&, WebCore::VisiblePosition const&) (WebCore:x86_64+0xef0087)
    #4 0x7fff4190afe1 in WebCore::ApplyBlockElementCommand::doApply() (WebCore:x86_64+0xeeefe1)
    #5 0x7fff40b931d9 in WebCore::CompositeEditCommand::apply() (WebCore:x86_64+0x1771d9)
    #6 0x7fff41953129 in WebCore::executeIndent(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (WebCore:x86_64+0xf37129)
    #7 0x7fff40bdcc3e in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (WebCore:x86_64+0x1c0c3e)
    #8 0x7fff40bdc9da in WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*) (WebCore:x86_64+0x1c09da)
    #9 0x30570ec01176  (<unknown module>)
    #10 0x7fff3706bbdb in llint_entry (JavaScriptCore:x86_64+0x3babdb)
    #11 0x7fff3705c738 in vmEntryToJavaScript (JavaScriptCore:x86_64+0x3ab738)
    #12 0x7fff36ceb492 in JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (JavaScriptCore:x86_64+0x3a492)
Comment 1 Jack 2020-03-05 00:48:12 PST
Root cause:

In function IndentOutdentCommand::indentIntoBlockquote, startOfContents is the parent of end and outerBlock, causing unexpected behavior in function CompositeEditCommand::cloneParagraphUnderNewElement.

In function cloneParagraphUnderNewElement, when outerBlock is not an ancestor of startOfContents, node traversal above the editable element, causing insertion of the cloned node to fail and the makes the node parentless. Later we try to insert another node to the parent of the cloned node, which is null, resulting in nullptr dereference.
Comment 2 Jack 2020-03-05 01:26:57 PST
Created attachment 392547 [details]
Comment 3 Jack 2020-03-05 01:32:36 PST
Created attachment 392548 [details]
Comment 4 Jack 2020-03-05 01:42:27 PST
Comment on attachment 392548 [details]

[121/1819] editing/execCommand/4641880-2.html failed unexpectedly (text diff)
stopping WebKitTestRunner(pid 51788) timed out, killing it                       
[391/1819] editing/execCommand/5481523.html failed unexpectedly (text diff)                          
[491/1819] editing/execCommand/5658933-1.html failed unexpectedly (text diff)      
[509/1819] editing/execCommand/5658933-2.html failed unexpectedly (text diff)      
[519/1819] editing/execCommand/5658933-3.html failed unexpectedly (text diff)         
[898/1819] editing/execCommand/indent-div-inside-list.html failed unexpectedly (text diff)          
[909/1819] editing/execCommand/indent-images-2.html failed unexpectedly (text diff)
[917/1819] editing/execCommand/indent-images.html failed unexpectedly (text diff)
[923/1819] editing/execCommand/indent-img-twice.html failed unexpectedly (text diff)
Comment 5 Jack 2020-03-05 02:48:40 PST
Created attachment 392552 [details]
Comment 6 Jack 2020-03-05 03:02:54 PST
Ran tests in LayoutTests/editing:

1815 tests ran as expected, 4 didn't:

Expected to fail, but passed: (4)

(In reply to Jack from comment #5)
> Created attachment 392552 [details]
> Patch
Comment 7 Jack 2020-03-08 08:39:36 PDT
The modified function is not called in the two failed test cases.
Comment 8 Ryosuke Niwa 2020-03-10 15:53:15 PDT
Comment on attachment 392552 [details]

Looks sane to me.
Comment 9 Ryosuke Niwa 2020-03-10 15:53:37 PDT
This is not a security bug.
Comment 10 WebKit Commit Bot 2020-03-10 16:41:23 PDT
Comment on attachment 392552 [details]

Clearing flags on attachment: 392552

Committed r258239: <https://trac.webkit.org/changeset/258239>
Comment 11 WebKit Commit Bot 2020-03-10 16:41:24 PDT
All reviewed patches have been landed.  Closing bug.