Title: [130266] trunk
Revision
130266
Author
[email protected]
Date
2012-10-03 02:23:30 -0700 (Wed, 03 Oct 2012)

Log Message

AX: Heap-use-after-free when deleting a ContainerNode with an AX object
https://bugs.webkit.org/show_bug.cgi?id=98073

Reviewed by Hajime Morita.

Source/WebCore:

Calls axObjectCache()->remove(this) in ~ContainerNode so that the AX tree
doesn't try to access the container node while walking up the parent chain
from one of the container node's children.

Test: accessibility/container-node-delete-causes-crash.html

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::~ContainerNode):
* dom/Node.cpp:
(WebCore::Node::~Node):
* dom/Node.h:
(WebCore::Node::document):
(WebCore::Node::documentInternal):

LayoutTests:

Adds test for heap-use-after-free when container node with AX object is deleted.

* accessibility/container-node-delete-causes-crash-expected.txt: Added.
* accessibility/container-node-delete-causes-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (130265 => 130266)


--- trunk/LayoutTests/ChangeLog	2012-10-03 09:03:44 UTC (rev 130265)
+++ trunk/LayoutTests/ChangeLog	2012-10-03 09:23:30 UTC (rev 130266)
@@ -1,3 +1,15 @@
+2012-10-03  Dominic Mazzoni  <[email protected]>
+
+        AX: Heap-use-after-free when deleting a ContainerNode with an AX object
+        https://bugs.webkit.org/show_bug.cgi?id=98073
+
+        Reviewed by Hajime Morita.
+
+        Adds test for heap-use-after-free when container node with AX object is deleted.
+
+        * accessibility/container-node-delete-causes-crash-expected.txt: Added.
+        * accessibility/container-node-delete-causes-crash.html: Added.
+
 2012-10-03  Vsevolod Vlasov  <[email protected]>
 
         Web Inspector: SourceURL should be taken from debugger agent when possible.

Added: trunk/LayoutTests/accessibility/container-node-delete-causes-crash-expected.txt (0 => 130266)


--- trunk/LayoutTests/accessibility/container-node-delete-causes-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/container-node-delete-causes-crash-expected.txt	2012-10-03 09:23:30 UTC (rev 130266)
@@ -0,0 +1,10 @@
+Checks to make sure a heap-use-after-free crash doesn't occur when a container node with an associated accessibility object is deleted from the tree. The heap-use-after free was occuring when the AccessibilityObject corresponding to the child of the text node walked up its parent chain in AccessibilityObject::supportsARIALiveRegion but its parent was already deleted.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Text
+

Added: trunk/LayoutTests/accessibility/container-node-delete-causes-crash.html (0 => 130266)


--- trunk/LayoutTests/accessibility/container-node-delete-causes-crash.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/container-node-delete-causes-crash.html	2012-10-03 09:23:30 UTC (rev 130266)
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<script src=""
+
+<div id="console"></div>
+
+<svg xmlns:xlink="http://www.w3.org/1999/xlink">
+  <text id="a">Text</text>
+  <use xlink:href=""
+</svg>
+
+<script>
+description("Checks to make sure a heap-use-after-free crash doesn't occur when a container node with an associated accessibility object is deleted from the tree. The heap-use-after free was occuring when the AccessibilityObject corresponding to the child of the text node walked up its parent chain in AccessibilityObject::supportsARIALiveRegion but its parent was already deleted.");
+
+// This creates an accessibility object for every node in the tree.
+if (window.accessibilityController)
+    accessibilityController.accessibleElementById("foo");
+
+// An SVG "use" element is like a clone, so the "use" element contains a
+// clone of the "text" element. This statement clears the reference, which
+// causes the cloned "text" element to be destroyed.
+document.getElementsByTagName('use')[0].setAttribute('xlink:href', '');
+</script>
+
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (130265 => 130266)


--- trunk/Source/WebCore/ChangeLog	2012-10-03 09:03:44 UTC (rev 130265)
+++ trunk/Source/WebCore/ChangeLog	2012-10-03 09:23:30 UTC (rev 130266)
@@ -1,3 +1,24 @@
+2012-10-03  Dominic Mazzoni  <[email protected]>
+
+        AX: Heap-use-after-free when deleting a ContainerNode with an AX object
+        https://bugs.webkit.org/show_bug.cgi?id=98073
+
+        Reviewed by Hajime Morita.
+
+        Calls axObjectCache()->remove(this) in ~ContainerNode so that the AX tree
+        doesn't try to access the container node while walking up the parent chain
+        from one of the container node's children.
+
+        Test: accessibility/container-node-delete-causes-crash.html
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::~ContainerNode):
+        * dom/Node.cpp:
+        (WebCore::Node::~Node):
+        * dom/Node.h:
+        (WebCore::Node::document):
+        (WebCore::Node::documentInternal):
+
 2012-10-03  Vsevolod Vlasov  <[email protected]>
 
         Web Inspector: SourceURL should be taken from debugger agent when possible.

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (130265 => 130266)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2012-10-03 09:03:44 UTC (rev 130265)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2012-10-03 09:23:30 UTC (rev 130266)
@@ -23,6 +23,7 @@
 #include "config.h"
 #include "ContainerNode.h"
 
+#include "AXObjectCache.h"
 #include "ChildListMutationScope.h"
 #include "ContainerNodeAlgorithms.h"
 #include "DeleteButtonController.h"
@@ -119,6 +120,9 @@
 
 ContainerNode::~ContainerNode()
 {
+    if (AXObjectCache::accessibilityEnabled() && documentInternal() && documentInternal()->axObjectCacheExists())
+        documentInternal()->axObjectCache()->remove(this);
+
     removeAllChildren();
 }
 

Modified: trunk/Source/WebCore/dom/Node.cpp (130265 => 130266)


--- trunk/Source/WebCore/dom/Node.cpp	2012-10-03 09:03:44 UTC (rev 130265)
+++ trunk/Source/WebCore/dom/Node.cpp	2012-10-03 09:23:30 UTC (rev 130266)
@@ -417,7 +417,7 @@
         detach();
 
     Document* doc = m_document;
-    if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists())
+    if (AXObjectCache::accessibilityEnabled() && doc && doc->axObjectCacheExists() && !isContainerNode())
         doc->axObjectCache()->remove(this);
     
     if (m_previous)

Modified: trunk/Source/WebCore/dom/Node.h (130265 => 130266)


--- trunk/Source/WebCore/dom/Node.h	2012-10-03 09:03:44 UTC (rev 130265)
+++ trunk/Source/WebCore/dom/Node.h	2012-10-03 09:23:30 UTC (rev 130266)
@@ -429,8 +429,8 @@
         ASSERT(this);
         // FIXME: below ASSERT is useful, but prevents the use of document() in the constructor or destructor
         // due to the virtual function call to nodeType().
-        ASSERT(m_document || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument()));
-        return m_document;
+        ASSERT(documentInternal() || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument()));
+        return documentInternal();
     }
 
     TreeScope* treeScope() const;
@@ -755,6 +755,8 @@
 
     void setHasCustomCallbacks() { setFlag(true, HasCustomCallbacksFlag); }
 
+    Document* documentInternal() const { return m_document; }
+
 private:
     friend class TreeShared<Node, ContainerNode>;
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to