Bug 225235 - Crash in ApplyStyleCommand::applyRelativeFontStyleChange
Summary: Crash in ApplyStyleCommand::applyRelativeFontStyleChange
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-30 09:21 PDT by Ali Juma
Modified: 2021-05-07 08:24 PDT (History)
10 users (show)

See Also:


Attachments
Minimal test case (1.35 KB, text/html)
2021-04-30 09:21 PDT, Ali Juma
no flags Details
testcase reduced a bit further (696 bytes, text/html)
2021-05-03 03:19 PDT, Frédéric Wang (:fredw)
no flags Details
experimental patch (788 bytes, patch)
2021-05-03 11:47 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (4.42 KB, patch)
2021-05-04 00:05 PDT, Frédéric Wang (:fredw)
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (5.00 KB, patch)
2021-05-04 03:52 PDT, Frédéric Wang (:fredw)
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (4.93 KB, patch)
2021-05-07 00:51 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ali Juma 2021-04-30 09:21:34 PDT
Created attachment 427430 [details]
Minimal test case

Filing this as a security bug since it was found using a fuzzer; there's no disclosure deadline for this bug.

This reproduces in an ASan build of WebKitTestRunner, and also in STP 123.

Stack:
=================================================================
==29379==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000018 (pc 0x0005c83e7854 bp 0x7ffee1561650 sp 0x7ffee1561650 T0)
==29379==The signal is caused by a WRITE memory access.
==29379==Hint: address points to the zero page.
    #0 0x5c83e7853 in WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >::Ref(WebCore::Node&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x484853)
    #1 0x5cb7df0bf in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange(WebCore::EditingStyle*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x387c0bf)
    #2 0x5cb7dda06 in WebCore::ApplyStyleCommand::doApply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x387aa06)
    #3 0x5cb7d80c4 in WebCore::CompositeEditCommand::apply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x38750c4)
    #4 0x5cb84ea01 in WebCore::Editor::applyStyle(WTF::RefPtr<WebCore::EditingStyle, WTF::RawPtrTraits<WebCore::EditingStyle>, WTF::DefaultRefDerefTraits<WebCore::EditingStyle> >&&, WebCore::EditAction, WebCore::Editor::ColorFilterMode) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x38eba01)
    #5 0x5cb897175 in WebCore::applyCommandToFrame(WebCore::Frame&, WebCore::EditorCommandSource, WebCore::EditAction, WTF::Ref<WebCore::EditingStyle, WTF::RawPtrTraits<WebCore::EditingStyle> >&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3934175)
    #6 0x5cb897016 in WebCore::executeApplyStyle(WebCore::Frame&, WebCore::EditorCommandSource, WebCore::EditAction, WebCore::CSSPropertyID, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3934016)
    #7 0x5cb4f96a3 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x35966a3)
    #8 0x5c894120a in WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9de20a)
    #9 0x5c8940ccb in long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9ddccb)
    #10 0x24a3a7a011d7  (<unknown module>)
    #11 0x5e4a307df in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb4e7df)
    #12 0x5e4a307df in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb4e7df)
    #13 0x5e4a157f8 in vmEntryToJavaScript (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb337f8)
    #14 0x5e61b9c61 in JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x22d7c61)
    #15 0x5e68fa7ed in JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2a187ed)
    #16 0x5e68faa97 in JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2a18a97)
    #17 0x5cae12bb9 in WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2eafbb9)
    #18 0x5cae123b9 in WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2eaf3b9)
    #19 0x5cae11fad in WebCore::ScriptController::evaluateInWorldIgnoringException(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2eaefad)
    #20 0x5cb6f93f7 in WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x37963f7)
    #21 0x5cb6f65eb in WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x37935eb)
    #22 0x5cbe3196e in WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ece96e)
    #23 0x5cbe31644 in WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::RawPtrTraits<WebCore::ScriptElement> >&&, WTF::TextPosition const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ece644)
    #24 0x5cbe0a449 in WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea7449)
    #25 0x5cbe0aa8d in WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea7a8d)
    #26 0x5cbe09b2b in WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea6b2b)
    #27 0x5cbe09f9d in WebCore::HTMLDocumentParser::resumeParsingAfterYield() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3ea6f9d)
    #28 0x5cc8aa4a4 in WebCore::ThreadTimers::sharedTimerFiredInternal() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x49474a4)
    #29 0x5cc934109 in WebCore::timerFired(__CFRunLoopTimer*, void*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x49d1109)
    #30 0x7fff31204467 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9f467)
    #31 0x7fff31203fcd in __CFRunLoopDoTimer (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9efcd)
    #32 0x7fff31203ab8 in __CFRunLoopDoTimers (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9eab8)
    #33 0x7fff311e870c in __CFRunLoopRun (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x8370c)
    #34 0x7fff311e7952 in CFRunLoopRunSpecific (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x82952)
    #35 0x7fff338a51c7 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x601c7)
    #36 0x7fff33957c6e in -[NSRunLoop(NSRunLoop) run] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x112c6e)
    #37 0x7fff6b3c44e9 in _xpc_objc_main.cold.4 (/usr/lib/system/libxpc.dylib:x86_64+0x164e9)
    #38 0x7fff6b3c442f in _xpc_objc_main (/usr/lib/system/libxpc.dylib:x86_64+0x1642f)
    #39 0x7fff6b3c3f62 in xpc_main (/usr/lib/system/libxpc.dylib:x86_64+0x15f62)
    #40 0x5b8f119f3 in WebKit::XPCServiceMain(int, char const**) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebKit.framework/Versions/A/WebKit:x86_64+0xf119f3)
    #41 0x7fff6b172cc8 in start (/usr/lib/system/libdyld.dylib:x86_64+0x1acc8)
==29379==Register values:
rax = 0x0000100000000000  rbx = 0x00007ffee1561aa0  rcx = 0x0000100000000003  rdx = 0x0000000000000003
rdi = 0x00007ffee1561860  rsi = 0x0000000000000018  rbp = 0x00007ffee1561650  rsp = 0x00007ffee1561650
 r8 = 0x0000000000000001   r9 = 0x0000000000000000  r10 = 0xffffffffffffffff  r11 = 0x00000fffffffffff
r12 = 0x00007ffee1561840  r13 = 0x00001fffdc2ac310  r14 = 0x00007ffee1561860  r15 = 0x00001fffdc2ac2cc
=========================
Clusterfuzz-id: 5723658426056704
Comment 1 Radar WebKit Bug Importer 2021-04-30 09:21:44 PDT
<rdar://problem/77385816>
Comment 2 Ryosuke Niwa 2021-04-30 21:14:22 PDT
Frédéric, is this a duplicate of the bug 223974?
Comment 3 Frédéric Wang (:fredw) 2021-05-03 02:06:32 PDT
(In reply to Ryosuke Niwa from comment #2)
> Frédéric, is this a duplicate of the bug 223974?

The pacth there does not fix it and the assertion failure seems different. Will take a look.
Comment 4 Frédéric Wang (:fredw) 2021-05-03 03:19:10 PDT
Created attachment 427548 [details]
testcase reduced a bit further
Comment 5 Frédéric Wang (:fredw) 2021-05-03 08:06:42 PDT
(In reply to Frédéric Wang (:fredw) from comment #3)
> The pacth there does not fix it and the assertion failure seems different.
> Will take a look.

Actually it's the same debug assertion https://webkit-search.igalia.com/webkit/rev/d6c003baa462e89279b24c5ebfe160082a97997e/Source/WebCore/editing/ApplyStyleCommand.cpp#384 but the reason is different.

start/end are both pointing to the body with offsets 2 and 4 respectively, so we have start < end. The inversion of nodes happens at https://webkit-search.igalia.com/webkit/rev/d6c003baa462e89279b24c5ebfe160082a97997e/Source/WebCore/editing/ApplyStyleCommand.cpp#345 where "beyondEnd = NodeTraversal::next(*end.deprecatedNode());" moves to the DIV child while "start = start.upstream();" moves to the #text child. I'm not sure why we use two different functions?

(rr) b Source/WebCore/editing/ApplyStyleCommand.cpp:356
(rr) rc
(rr) 
Thread 1 hit Breakpoint 1, WebCore::ApplyStyleCommand::applyRelativeFontStyleChange (this=0x7fde709b7bc0, style=0x7fde709a3cf0)
    at https://webkit-search.igalia.com/webkit/rev/d6c003baa462e89279b24c5ebfe160082a97997e/Source/WebCore/editing/ApplyStyleCommand.cpp#356
(rr) p showTree(start)
*BODY	0x7fdebab08010 (renderer 0x7fdebab11b40)  STYLE=-webkit-text-security: disc;
	DIV	0x7fdebab08110 (renderer 0x7fdebab08640) 
	#text	0x7fdebab09330 "B\n  "
	SCRIPT	0x7fdebab093a0 (renderer (nil)) 
		#text	0x7fdebab080a0 "\n    document.body.insertBefore(document.createElement("div"),\n                               document.body.firstChild);\n    document.execCommand("FontSizeDelta", false, 3);\n  "
offset, offset:2
$1 = void
(rr) p showTree(end)
*BODY	0x7fdebab08010 (renderer 0x7fdebab11b40)  STYLE=-webkit-text-security: disc;
	DIV	0x7fdebab08110 (renderer 0x7fdebab08640) 
	#text	0x7fdebab09330 "B\n  "
	SCRIPT	0x7fdebab093a0 (renderer (nil)) 
		#text	0x7fdebab080a0 "\n    document.body.insertBefore(document.createElement("div"),\n                               document.body.firstChild);\n    document.execCommand("FontSizeDelta", false, 3);\n  "
offset, offset:4
$2 = void
(rr) p showTree(&*beyondEnd)
BODY	0x7fdebab08010 (renderer 0x7fdebab11b40)  STYLE=-webkit-text-security: disc;
*	DIV	0x7fdebab08110 (renderer 0x7fdebab08640) 
	#text	0x7fdebab09330 "B\n  "
	SCRIPT	0x7fdebab093a0 (renderer (nil)) 
		#text	0x7fdebab080a0 "\n    document.body.insertBefore(document.createElement("div"),\n                               document.body.firstChild);\n    document.execCommand("FontSizeDelta", false, 3);\n  "
$3 = void
(rr) n
(rr) p showTree(start)
BODY	0x7fdebab08010 (renderer 0x7fdebab11b40)  STYLE=-webkit-text-security: disc;
	DIV	0x7fdebab08110 (renderer 0x7fdebab08640) 
*	#text	0x7fdebab09330 "B\n  "
	SCRIPT	0x7fdebab093a0 (renderer (nil)) 
		#text	0x7fdebab080a0 "\n    document.body.insertBefore(document.createElement("div"),\n                               document.body.firstChild);\n    document.execCommand("FontSizeDelta", false, 3);\n  "
legacy, offset, offset:4
$4 = void
Comment 6 Frédéric Wang (:fredw) 2021-05-03 11:47:56 PDT
Created attachment 427582 [details]
experimental patch

This ensures beyondEnd is moved to the next node, if the start & end have the same anchor node. I don't know if this is what we want or not, I'm uploading it to see whether it causes any test failures on EWS.

(In any case when the anchor node is the same, using NodeTraversal::next() on the end's anchor node and Position::upstream() on the selection start seems wrong)
Comment 7 Ryosuke Niwa 2021-05-03 18:11:43 PDT
Comment on attachment 427582 [details]
experimental patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427582&action=review

It doesn't seem like this is a security issue? Can we add a test?

> Source/WebCore/editing/ApplyStyleCommand.cpp:351
> +    if (start.deprecatedNode() == end.deprecatedNode() || start.deprecatedNode()->isDescendantOf(*end.deprecatedNode()))

Sounds like we want to do do: end.deprecatedNode()->contains(*start.deprecatedNode())?
Comment 8 Frédéric Wang (:fredw) 2021-05-04 00:05:00 PDT
Created attachment 427641 [details]
Patch

(In reply to Ryosuke Niwa from comment #7)
> It doesn't seem like this is a security issue? Can we add a test?

Yes, the first loop will crash with a nullptr dereference when reaching the end of the DOM tree, similar to the debug assert.

Here is a new version, including a test. Unfortunately, I was not able to reduce it much.
Comment 9 Frédéric Wang (:fredw) 2021-05-04 03:52:56 PDT
Created attachment 427649 [details]
Patch
Comment 10 Ryosuke Niwa 2021-05-06 11:21:28 PDT
Comment on attachment 427649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427649&action=review

> LayoutTests/editing/execCommand/font-size-delta-same-node-for-start-and-end-crash.html:19
> +    document.body.insertBefore(document.createElement("div"),
> +                               document.body.firstChild);

Nit: wrong indentation. Our indentation is always 4 spaces.
Comment 11 Ryosuke Niwa 2021-05-06 11:22:05 PDT
Comment on attachment 427649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427649&action=review

> LayoutTests/editing/execCommand/font-size-delta-same-node-for-start-and-end-crash.html:1
> +<body style="-webkit-text-security: disc;">

Do we really need this?
Comment 12 Frédéric Wang (:fredw) 2021-05-06 11:27:52 PDT
Comment on attachment 427649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427649&action=review

>> LayoutTests/editing/execCommand/font-size-delta-same-node-for-start-and-end-crash.html:1
>> +<body style="-webkit-text-security: disc;">
> 
> Do we really need this?

Crash does not happen without that (I think it's creating text nodes). In general I was not really able to reduce further.
Comment 13 Ryosuke Niwa 2021-05-06 11:29:19 PDT
(In reply to Frédéric Wang (:fredw) from comment #12)
> Comment on attachment 427649 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=427649&action=review
> 
> >> LayoutTests/editing/execCommand/font-size-delta-same-node-for-start-and-end-crash.html:1
> >> +<body style="-webkit-text-security: disc;">
> > 
> > Do we really need this?
> 
> Crash does not happen without that (I think it's creating text nodes). In
> general I was not really able to reduce further.

Okay. Interesting. I wonder how that CSS rule is affecting the editing code...
Comment 14 Frédéric Wang (:fredw) 2021-05-07 00:51:18 PDT
Created attachment 427977 [details]
Patch for landing
Comment 15 EWS 2021-05-07 08:23:59 PDT
Committed r277174 (237460@main): <https://commits.webkit.org/237460@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427977 [details].