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

Jack
Reported 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)
Attachments
Patch (4.16 KB, patch)
2020-03-05 01:26 PST, Jack
no flags
Patch (4.17 KB, patch)
2020-03-05 01:32 PST, Jack
no flags
Patch (4.33 KB, patch)
2020-03-05 02:48 PST, Jack
no flags
Jack
Comment 1 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.
Jack
Comment 2 2020-03-05 01:26:57 PST
Jack
Comment 3 2020-03-05 01:32:36 PST
Jack
Comment 4 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)
Jack
Comment 5 2020-03-05 02:48:40 PST
Jack
Comment 6 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
Jack
Comment 7 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
Ryosuke Niwa
Comment 8 2020-03-10 15:53:15 PDT
Comment on attachment 392552 [details] Patch Looks sane to me.
Ryosuke Niwa
Comment 9 2020-03-10 15:53:37 PDT
This is not a security bug.
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2020-03-10 16:41:24 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.