| Summary: | Nullptr crash in HTMLStackItem::create via DocumentFragment::parseHTML | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||||
| Component: | HTML Editing | Assignee: | Rob Buis <rbuis> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | bfulgham, cgarcia, darin, ews-feeder, ews-watchlist, fred.wang, gpoo, mifenton, product-security, rbuis, svillar, webkit-bug-importer, wenson_hsieh | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Ryosuke Niwa
2021-06-25 02:29:37 PDT
Created attachment 432248 [details]
Test
Created attachment 432327 [details]
Patch
Created attachment 432328 [details]
Patch
Comment on attachment 432328 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432328&action=review > Source/WebCore/page/DOMSelection.cpp:357 > + if (!rangeCount()) > + return Exception { InvalidStateError }; Given we had never thrown before in this case, I think it's too risky to do this. Let's just ignore and return { } instead at least for now. Created attachment 432349 [details]
Patch
Comment on attachment 432349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432349&action=review > Source/WebCore/ChangeLog:11 > + Selection.extend() should throw if rangeCount is 0 [1]. > + However as a first step just make it do nothing. > + > + Behavior matches Firefox and Chrome. Could you explain how this relates to the actual crash we're fixing? Document looks like this before calling parseHTML:
*#document 0x61f00005f680 (renderer 0x61700004db00)
HTML 0x60c0000ba4c0 (renderer 0x612000080d40)
#text 0x60b0000fd7d0 "<d800>"
DIV 0x60c0000bd940 (renderer 0x612000083140)
BR 0x60c0000bddc0 (renderer 0x6110000eeb00)
HEAD 0x60c0000ba580 (renderer 0x0)
SCRIPT 0x610000039340 (renderer 0x0)
#text 0x60b0000fcee0 "\n onload = () => {\n document.documentElement.prepend('\\ud800');\nconsole.log(document.head);\n getSelection().collapse(document.head, 1);\n getSelection().extend(document.head);\n document.designMode = 'on';\n document.execCommand('InsertParagraph');\n document.execCommand('SelectAll');\n document.execCommand('Copy');\n document.execCommand('Paste');\n };\n"
#text 0x60b0000fd040 "\n"
BODY 0x60c0000bb000 (renderer 0x612000083a40)
Comment on attachment 432349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432349&action=review >> Source/WebCore/ChangeLog:11 >> + Behavior matches Firefox and Chrome. > > Could you explain how this relates to the actual crash we're fixing? Looking at it again, it is probably a workaround rather than a fix. Created attachment 432390 [details]
Patch
Created attachment 432399 [details]
Patch
Comment on attachment 432399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432399&action=review > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:149 > + // We use a fake body element here to trick the HTML parser into using the InBody insertion mode. > + auto fakeBody = HTMLBodyElement::create(document); This isn't really a fake element, right? It's just a dummy element to force InBody insertion mode. Instead of adding a comment like this, give it a longer variable name like this: auto dummyBodyToForceInBodyInsertionMode = ~ Comment on attachment 432399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432399&action=review > Source/WebCore/ChangeLog:3 > + Use fake body for parseHTML call Since this isn't a security bug, please make this patch the bug title. > Source/WebCore/ChangeLog:8 > + Use fake body for parseHTML call. Please explain what was causing the crash & how you fixed it. > LayoutTests/ChangeLog:3 > + Use fake body for parseHTML call Ditto. Created attachment 432462 [details]
Patch
Committed r279371 (239237@main): <https://commits.webkit.org/239237@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432462 [details]. |