- Revision
- 287812
- Author
- [email protected]
- Date
- 2022-01-08 10:21:55 -0800 (Sat, 08 Jan 2022)
Log Message
null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
https://bugs.webkit.org/show_bug.cgi?id=234862
Patch by Gabriel Nava Marino <[email protected]> on 2022-01-08
Reviewed by Darin Adler.
Source/WebCore:
ModifySelectionListLevelCommand::appendSiblingNodeRange loops through nodes assuming
existence of siblings, which is not guaranteed, and can result in nullptr deref. Instead,
check for node existence as part of loop condition, and change raw pointer usage to RefPtr.
This addresses the crash but results in ASSERT(isEndOfParagraph(endOfParagraphToMove))
failing in CompositeEditCommand::moveParagraph. We modify WebCore::findEndOfParagraph
to check for HTMLBRElement nodes to avoid unexpectedly changing the AnchorType.
Test: http/tests/lists/list-new-parent-no-sibling-append.html
* editing/ModifySelectionListLevel.cpp:
(WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore):
(WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter):
(WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange):
* editing/VisibleUnits.cpp:
(WebCore::findEndOfParagraph):
LayoutTests:
* http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
* http/tests/lists/list-new-parent-no-sibling-append.html: Added.
* platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
* platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (287811 => 287812)
--- trunk/LayoutTests/ChangeLog 2022-01-08 16:46:39 UTC (rev 287811)
+++ trunk/LayoutTests/ChangeLog 2022-01-08 18:21:55 UTC (rev 287812)
@@ -1,3 +1,15 @@
+2022-01-08 Gabriel Nava Marino <[email protected]>
+
+ null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
+ https://bugs.webkit.org/show_bug.cgi?id=234862
+
+ Reviewed by Darin Adler.
+
+ * http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
+ * http/tests/lists/list-new-parent-no-sibling-append.html: Added.
+ * platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
+ * platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
+
2022-01-08 Tyler Wilcock <[email protected]>
AX: AccessibilityObject::setFocused(true) should make the webpage focused, and make web content the first responder
Added: trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (0 => 287812)
--- trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append-expected.txt 2022-01-08 18:21:55 UTC (rev 287812)
@@ -0,0 +1,9 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin. Status code: 200
+
+
+
+
+
+
+
Added: trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append.html (0 => 287812)
--- trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append.html (rev 0)
+++ trunk/LayoutTests/http/tests/lists/list-new-parent-no-sibling-append.html 2022-01-08 18:21:55 UTC (rev 287812)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<style>
+ div {
+ aspect-ratio: 1;
+ mask-image: url(http://localhost:8000);
+ }
+ ol {
+ column-span: all;
+ }
+ ol:only-child {
+ column-count: 2;
+ }
+ :nth-child(2) {
+ content: url();
+ }
+</style>
+<script>
+ _onload_ = () => {
+ document.documentElement.append(document.createElement('div'));
+ document.designMode = 'on';
+ document.execCommand('SelectAll');
+ for (let i = 0; i < 10; i++) {
+ document.execCommand('InsertNestedOrderedList');
+ }
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ console.log("This test passes if it does not crash.");
+ };
+</script>
Added: trunk/LayoutTests/platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (0 => 287812)
--- trunk/LayoutTests/platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt 2022-01-08 18:21:55 UTC (rev 287812)
@@ -0,0 +1,9 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin. Status code: 404
+
+
+
+
+
+
+
Added: trunk/LayoutTests/platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (0 => 287812)
--- trunk/LayoutTests/platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt 2022-01-08 18:21:55 UTC (rev 287812)
@@ -0,0 +1,9 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin. Status code: 404
+
+
+
+
+
+
+
Modified: trunk/Source/WebCore/ChangeLog (287811 => 287812)
--- trunk/Source/WebCore/ChangeLog 2022-01-08 16:46:39 UTC (rev 287811)
+++ trunk/Source/WebCore/ChangeLog 2022-01-08 18:21:55 UTC (rev 287812)
@@ -1,3 +1,27 @@
+2022-01-08 Gabriel Nava Marino <[email protected]>
+
+ null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
+ https://bugs.webkit.org/show_bug.cgi?id=234862
+
+ Reviewed by Darin Adler.
+
+ ModifySelectionListLevelCommand::appendSiblingNodeRange loops through nodes assuming
+ existence of siblings, which is not guaranteed, and can result in nullptr deref. Instead,
+ check for node existence as part of loop condition, and change raw pointer usage to RefPtr.
+
+ This addresses the crash but results in ASSERT(isEndOfParagraph(endOfParagraphToMove))
+ failing in CompositeEditCommand::moveParagraph. We modify WebCore::findEndOfParagraph
+ to check for HTMLBRElement nodes to avoid unexpectedly changing the AnchorType.
+
+ Test: http/tests/lists/list-new-parent-no-sibling-append.html
+
+ * editing/ModifySelectionListLevel.cpp:
+ (WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore):
+ (WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter):
+ (WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange):
+ * editing/VisibleUnits.cpp:
+ (WebCore::findEndOfParagraph):
+
2022-01-08 Tyler Wilcock <[email protected]>
AX: Improve WeakHashSet hygienics in AXObjectCache
Modified: trunk/Source/WebCore/editing/ModifySelectionListLevel.cpp (287811 => 287812)
--- trunk/Source/WebCore/editing/ModifySelectionListLevel.cpp 2022-01-08 16:46:39 UTC (rev 287811)
+++ trunk/Source/WebCore/editing/ModifySelectionListLevel.cpp 2022-01-08 18:21:55 UTC (rev 287812)
@@ -92,48 +92,52 @@
void ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore(Node* startNode, Node* endNode, Node* refNode)
{
- Node* node = startNode;
- while (1) {
- Node* next = node->nextSibling();
+ RefPtr node = startNode;
+ while (node) {
+ RefPtr next = node->nextSibling();
removeNode(*node);
insertNodeBefore(*node, *refNode);
if (node == endNode)
- break;
+ return;
node = next;
}
+ ASSERT_NOT_REACHED();
}
void ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter(Node* startNode, Node* endNode, Node* refNode)
{
- Node* node = startNode;
- while (1) {
- Node* next = node->nextSibling();
+ RefPtr node = startNode;
+ RefPtr refChild = refNode;
+ while (node) {
+ RefPtr next = node->nextSibling();
removeNode(*node);
- insertNodeAfter(*node, *refNode);
+ insertNodeAfter(*node, *refChild);
if (node == endNode)
- break;
+ return;
- refNode = node;
+ refChild = node;
node = next;
}
+ ASSERT_NOT_REACHED();
}
void ModifySelectionListLevelCommand::appendSiblingNodeRange(Node* startNode, Node* endNode, Element* newParent)
{
- Node* node = startNode;
- while (1) {
- Node* next = node->nextSibling();
+ RefPtr node = startNode;
+ while (node) {
+ RefPtr next = node->nextSibling();
removeNode(*node);
appendNode(*node, *newParent);
if (node == endNode)
- break;
+ return;
node = next;
}
+ ASSERT_NOT_REACHED();
}
IncreaseSelectionListLevelCommand::IncreaseSelectionListLevelCommand(Document& document, Type listType)
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (287811 => 287812)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2022-01-08 16:46:39 UTC (rev 287811)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2022-01-08 18:21:55 UTC (rev 287812)
@@ -1196,7 +1196,7 @@
}
// FIXME: This is wrong when startNode is a block. We should return a position after the block.
- if (r->isBR() || isBlock(n))
+ if (r->isBR() || is<HTMLBRElement>(n) || isBlock(n))
break;
// FIXME: We avoid returning a position where the renderer can't accept the caret.
