Changeset 269137 in webkit

Timestamp:

Oct 28, 2020, 10:26:10 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

Null dereference in CompositeEditCommand::cloneParagraphUnderNewElement() due to not checking for top of DOM tree
​https://bugs.webkit.org/show_bug.cgi?id=218132

Patch by Julian Gonzalez <​julian_a_gonzalez@apple.com> on 2020-10-28
Reviewed by Ryosuke Niwa.

Source/WebCore:

When iterating through parent nodes, cloneParagraphUnderNewElement()
has to be careful to check for the top of the DOM tree (where
parentNode() returns nullptr) and stop iterating.

• editing/CompositeEditCommand.cpp:

(WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):

LayoutTests:

Add a test to verify that we don't iterate outside the DOM tree while cloning a paragraph.
Thanks to Ryosuke Niwa for helping minimize the test and make it more stable.

• editing/deleting/move-paragraph-crash-expected.txt: Added.
• editing/deleting/move-paragraph-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/deleting/move-paragraph-crash-expected.txt (added)
• LayoutTests/editing/deleting/move-paragraph-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/CompositeEditCommand.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-10-28  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Null dereference in CompositeEditCommand::cloneParagraphUnderNewElement() due to not checking for top of DOM tree
+        https://bugs.webkit.org/show_bug.cgi?id=218132
+
+        Reviewed by Ryosuke Niwa.
+
+        Add a test to verify that we don't iterate outside the DOM tree while cloning a paragraph.
+        Thanks to Ryosuke Niwa for helping minimize the test and make it more stable.
+
+        * editing/deleting/move-paragraph-crash-expected.txt: Added.
+        * editing/deleting/move-paragraph-crash.html: Added.
+
 2020-10-28  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-28  Julian Gonzalez  <julian_a_gonzalez@apple.com>
+
+        Null dereference in CompositeEditCommand::cloneParagraphUnderNewElement() due to not checking for top of DOM tree
+        https://bugs.webkit.org/show_bug.cgi?id=218132
+
+        Reviewed by Ryosuke Niwa.
+
+        When iterating through parent nodes, cloneParagraphUnderNewElement()
+        has to be careful to check for the top of the DOM tree (where
+        parentNode() returns nullptr) and stop iterating.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::cloneParagraphUnderNewElement):
+
 2020-10-28  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/editing/CompositeEditCommand.cpp

@@ -1269 +1269 @@
         // find the first common ancestor to increase the scope
         // of our nextSibling traversal.
-        while (!end.deprecatedNode()->isDescendantOf(outerNode.get())) {
+        while (!end.deprecatedNode()->isDescendantOf(outerNode.get()) && outerNode->parentNode()) {
             outerNode = outerNode->parentNode();
         }
@@ -1278 +1278 @@
             // tree by NodeTraversal::nextSkippingChildren, so that the relative depth between
             // node and the original start node is maintained in the clone.
-            while (startNode->parentNode() != node->parentNode()) {
+            while (startNode->parentNode() && startNode->parentNode() != node->parentNode()) {
                 startNode = startNode->parentNode();
                 lastNode = lastNode->parentNode();
+                ASSERT(lastNode);
             }