Bug 208628

Summary: Nullptr crash in CompositeEditCommand::moveParagraphWithClones when indenting non-enclosed elements.
Product: WebKit Reporter: Jack <shihchieh_lee>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, ews-feeder, product-security, rniwa, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Jack 2020-03-05 00:29:19 PST 
<rdar://52011509>

==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]
Patch
Comment 3 Jack 2020-03-05 01:32:36 PST 
Created attachment 392548 [details]
Patch
Comment 4 Jack 2020-03-05 01:42:27 PST 
Comment on attachment 392548 [details]
Patch

[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]
Patch
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)
  editing/mac/selection/context-menu-select-editability.html
  editing/spelling/spelling-marker-includes-hyphen.html
  editing/spelling/spelling-markers-in-overlapping-lines-large-font.html
  editing/spelling/spelling-markers-in-overlapping-lines.html

(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.
fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html
Comment 8 Ryosuke Niwa 2020-03-10 15:53:15 PDT 
Comment on attachment 392552 [details]
Patch

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]
Patch

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.