Nullptr crash in DeleteSelectionCommand::handleGeneralDelete
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jun 2019 04:04:23 +0000 (04:04 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jun 2019 04:04:23 +0000 (04:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199126

Reviewed by Wenson Hsieh.

Added null checks to handleGeneralDelete as well as mergeParagraphs which runs after handleGeneralDelete to be defensive.

Unfortunately no new tests since there is no reproducible test case.

* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::handleGeneralDelete):
(WebCore::DeleteSelectionCommand::mergeParagraphs):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246706 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/DeleteSelectionCommand.cpp

index ee2eed7..7792de9 100644 (file)
@@ -1,3 +1,18 @@
+2019-06-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Nullptr crash in DeleteSelectionCommand::handleGeneralDelete
+        https://bugs.webkit.org/show_bug.cgi?id=199126
+
+        Reviewed by Wenson Hsieh.
+
+        Added null checks to handleGeneralDelete as well as mergeParagraphs which runs after handleGeneralDelete to be defensive.
+
+        Unfortunately no new tests since there is no reproducible test case.
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleGeneralDelete):
+        (WebCore::DeleteSelectionCommand::mergeParagraphs):
+
 2019-06-21  Youenn Fablet  <youenn@apple.com>
 
         ResourceLoadNotifier should check whether its frame document loader is null
index ffb96e4..68d1506 100644 (file)
@@ -610,7 +610,9 @@ void DeleteSelectionCommand::handleGeneralDelete()
             }
         }
         
-        if (m_downstreamEnd.deprecatedNode() != startNode && !m_upstreamStart.deprecatedNode()->isDescendantOf(m_downstreamEnd.deprecatedNode()) && m_downstreamEnd.anchorNode()->isConnected() && m_downstreamEnd.deprecatedEditingOffset() >= caretMinOffset(*m_downstreamEnd.deprecatedNode())) {
+        if (!m_downstreamEnd.isNull() && !m_downstreamEnd.isOrphan() && m_downstreamEnd.deprecatedNode() != startNode
+            && !m_upstreamStart.deprecatedNode()->isDescendantOf(m_downstreamEnd.deprecatedNode())
+            && m_downstreamEnd.deprecatedEditingOffset() >= caretMinOffset(*m_downstreamEnd.deprecatedNode())) {
             if (m_downstreamEnd.atLastEditingPositionForNode() && !canHaveChildrenForEditing(*m_downstreamEnd.deprecatedNode())) {
                 // The node itself is fully selected, not just its contents.  Delete it.
                 removeNode(*m_downstreamEnd.deprecatedNode());
@@ -679,7 +681,7 @@ void DeleteSelectionCommand::mergeParagraphs()
     ASSERT(!m_pruneStartBlockIfNecessary);
 
     // FIXME: Deletion should adjust selection endpoints as it removes nodes so that we never get into this state (4099839).
-    if (!m_downstreamEnd.anchorNode()->isConnected() || !m_upstreamStart.anchorNode()->isConnected())
+    if (m_downstreamEnd.isNull() || m_upstreamStart.isNull() || !m_downstreamEnd.anchorNode()->isConnected() || !m_upstreamStart.anchorNode()->isConnected())
          return;
 
     // FIXME: The deletion algorithm shouldn't let this happen.