Use-after-free in DOMSelection::containsNode
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 May 2013 02:36:37 +0000 (02:36 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 May 2013 02:36:37 +0000 (02:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=116468

Reviewed by Andreas Kling.

Source/WebCore:

Retain the node pointer. Also bail out early if the node was not in the document
since Range::compareBoundaryPoints sets ec to WRONG_DOCUMENT_ERR otherwise.

Test: editing/selection/contains-node-crash.html

* page/DOMSelection.cpp:
(WebCore::DOMSelection::containsNode):
* page/DOMSelection.h:
(DOMSelection):

LayoutTests:

Add a regression test from https://chromium.googlesource.com/chromium/blink/+/40bb8089352b15dd034641b4c131111cd79b44f1.

* editing/selection/contains-node-crash-expected.txt: Added.
* editing/selection/contains-node-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/contains-node-crash-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/contains-node-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/DOMSelection.cpp
Source/WebCore/page/DOMSelection.h

index 6ca258e..16962af 100644 (file)
@@ -1,3 +1,15 @@
+2013-05-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Use-after-free in DOMSelection::containsNode
+        https://bugs.webkit.org/show_bug.cgi?id=116468
+
+        Reviewed by Andreas Kling.
+
+        Add a regression test from https://chromium.googlesource.com/chromium/blink/+/40bb8089352b15dd034641b4c131111cd79b44f1.
+
+        * editing/selection/contains-node-crash-expected.txt: Added.
+        * editing/selection/contains-node-crash.html: Added.
+
 2013-05-21  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Unreviewed EFL gardening.
diff --git a/LayoutTests/editing/selection/contains-node-crash-expected.txt b/LayoutTests/editing/selection/contains-node-crash-expected.txt
new file mode 100644 (file)
index 0000000..d926522
--- /dev/null
@@ -0,0 +1,2 @@
+ALERT: PASS: containsNode returned false
+Test calling containsNode on a selection of a closed document doesn't cause a crash.
diff --git a/LayoutTests/editing/selection/contains-node-crash.html b/LayoutTests/editing/selection/contains-node-crash.html
new file mode 100644 (file)
index 0000000..35423f0
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html style="display: inline-table">
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+window.onload = function() {
+    var selection = getSelection();
+    selection.selectAllChildren(document);
+    selection.collapseToEnd();
+    var element = document.activeElement;
+    document.open();
+    document.write("Test calling containsNode on a selection of a closed document doesn't cause a crash.");
+    document.close();
+    if (selection.containsNode(element, true))
+        alert('FAIL: containsNode returned true even though the element was not in the document');
+    else
+        alert('PASS: containsNode returned false');
+    testRunner.notifyDone();
+}
+</script>
+</html>
index c22891c..76c6b84 100644 (file)
@@ -1,3 +1,20 @@
+2013-05-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Use-after-free in DOMSelection::containsNode
+        https://bugs.webkit.org/show_bug.cgi?id=116468
+
+        Reviewed by Andreas Kling.
+
+        Retain the node pointer. Also bail out early if the node was not in the document
+        since Range::compareBoundaryPoints sets ec to WRONG_DOCUMENT_ERR otherwise.
+
+        Test: editing/selection/contains-node-crash.html
+
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::containsNode):
+        * page/DOMSelection.h:
+        (DOMSelection):
+
 2013-05-21  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: InspectorFrontendHost::loadResourceSynchronously() builds ASCII-only results
index 76789b4..79c8a1f 100644 (file)
@@ -443,7 +443,7 @@ void DOMSelection::deleteFromDocument()
     setBaseAndExtent(selectedRange->startContainer(ASSERT_NO_EXCEPTION), selectedRange->startOffset(), selectedRange->startContainer(), selectedRange->startOffset(), ASSERT_NO_EXCEPTION);
 }
 
-bool DOMSelection::containsNode(const Node* n, bool allowPartial) const
+bool DOMSelection::containsNode(Node* n, bool allowPartial) const
 {
     if (!m_frame)
         return false;
@@ -453,12 +453,13 @@ bool DOMSelection::containsNode(const Node* n, bool allowPartial) const
     if (!n || m_frame->document() != n->document() || selection->isNone())
         return false;
 
-    ContainerNode* parentNode = n->parentNode();
-    unsigned nodeIndex = n->nodeIndex();
+    RefPtr<Node> node = n;
     RefPtr<Range> selectedRange = selection->selection().toNormalizedRange();
 
-    if (!parentNode)
+    ContainerNode* parentNode = node->parentNode();
+    if (!parentNode || !parentNode->inDocument())
         return false;
+    unsigned nodeIndex = node->nodeIndex();
 
     ExceptionCode ec = 0;
     bool nodeFullySelected = Range::compareBoundaryPoints(parentNode, nodeIndex, selectedRange->startContainer(), selectedRange->startOffset(), ec) >= 0 && !ec
@@ -473,7 +474,7 @@ bool DOMSelection::containsNode(const Node* n, bool allowPartial) const
     if (nodeFullyUnselected)
         return false;
 
-    return allowPartial || n->isTextNode();
+    return allowPartial || node->isTextNode();
 }
 
 void DOMSelection::selectAllChildren(Node* n, ExceptionCode& ec)
index 76b1b46..1dda6ab 100644 (file)
@@ -84,7 +84,7 @@ namespace WebCore {
         void removeAllRanges();
         void addRange(Range*);
         void deleteFromDocument();
-        bool containsNode(const Node*, bool partlyContained) const;
+        bool containsNode(Node*, bool partlyContained) const;
         void selectAllChildren(Node*, ExceptionCode&);
 
         String toString();