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