XPath::Step::nodesInAxis(): add null checks after Attr::ownerElement() calls
https://bugs.webkit.org/show_bug.cgi?id=235500
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
Import WPT tests from https://github.com/web-platform-tests/wpt/pull/32544.
* web-platform-tests/domxpath/xpath-evaluate-crash-expected.txt: Added.
* web-platform-tests/domxpath/xpath-evaluate-crash.html: Added.
Source/WebCore:
This patch adds null checks for results of Attr::ownerElement() to avoid crashes
when evaluating XPath expressions with an orphaned Attr as the context node.
Inspired by the recent Blink fix [1], yet this change covers all null pointer
dereferencing sites, as proven by the updated test.
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=1236967
Test: imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash.html
* xml/XPathStep.cpp:
(WebCore::XPath::Step::nodesInAxis const):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288589 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
index 694d051..0cc1987 100644
--- a/LayoutTests/imported/w3c/ChangeLog
+++ b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,15 @@
+2022-01-25 Alexey Shvayka <ashvayka@apple.com>
+
+ XPath::Step::nodesInAxis(): add null checks after Attr::ownerElement() calls
+ https://bugs.webkit.org/show_bug.cgi?id=235500
+
+ Reviewed by Darin Adler.
+
+ Import WPT tests from https://github.com/web-platform-tests/wpt/pull/32544.
+
+ * web-platform-tests/domxpath/xpath-evaluate-crash-expected.txt: Added.
+ * web-platform-tests/domxpath/xpath-evaluate-crash.html: Added.
+
2022-01-25 Antti Koivisto <antti@apple.com>
[CSS Container Queries] Parsing support for container shorthand property
diff --git a/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash-expected.txt
new file mode 100644
index 0000000..1f7f9b8
--- /dev/null
+++ b/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash-expected.txt
@@ -0,0 +1,3 @@
+
+PASS Evaluating XPath expressions with orhpaned Attr as context node doesn't crash
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash.html b/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash.html
new file mode 100644
index 0000000..3e1eaa5
--- /dev/null
+++ b/LayoutTests/imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Evaluating XPath expressions with orhpaned Attr as context node doesn't crash</title>
+<link rel=author href="mailto:jarhar@chromium.org">
+<link rel=help href="https://bugs.chromium.org/p/chromium/issues/detail?id=1236967">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script>
+test(() => {
+for (const expression of [
+ "..",
+ "parent",
+ "ancestor::*",
+ "ancestor-or-self::*",
+ "following::*",
+ "preceding::*",
+]) {
+ const orphanedAttr = document.createAttribute("foo");
+ new XPathEvaluator().evaluate(expression, orphanedAttr, null, 2);
+}
+});
+</script>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 770027a..f07d6ca 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2022-01-25 Alexey Shvayka <ashvayka@apple.com>
+
+ XPath::Step::nodesInAxis(): add null checks after Attr::ownerElement() calls
+ https://bugs.webkit.org/show_bug.cgi?id=235500
+
+ Reviewed by Darin Adler.
+
+ This patch adds null checks for results of Attr::ownerElement() to avoid crashes
+ when evaluating XPath expressions with an orphaned Attr as the context node.
+
+ Inspired by the recent Blink fix [1], yet this change covers all null pointer
+ dereferencing sites, as proven by the updated test.
+
+ [1] https://bugs.chromium.org/p/chromium/issues/detail?id=1236967
+
+ Test: imported/w3c/web-platform-tests/domxpath/xpath-evaluate-crash.html
+
+ * xml/XPathStep.cpp:
+ (WebCore::XPath::Step::nodesInAxis const):
+
2022-01-25 Simon Fraser <simon.fraser@apple.com>
Fix some spelling errors in Color functions
diff --git a/Source/WebCore/xml/XPathStep.cpp b/Source/WebCore/xml/XPathStep.cpp
index 105586a..29e1a67 100644
--- a/Source/WebCore/xml/XPathStep.cpp
+++ b/Source/WebCore/xml/XPathStep.cpp
@@ -258,7 +258,7 @@
case ParentAxis:
if (context.isAttributeNode()) {
Element* node = static_cast<Attr&>(context).ownerElement();
- if (nodeMatches(*node, ParentAxis, m_nodeTest))
+ if (node && nodeMatches(*node, ParentAxis, m_nodeTest))
nodes.append(node);
} else {
ContainerNode* node = context.parentNode();
@@ -270,6 +270,8 @@
Node* node = &context;
if (context.isAttributeNode()) {
node = static_cast<Attr&>(context).ownerElement();
+ if (!node)
+ return;
if (nodeMatches(*node, AncestorAxis, m_nodeTest))
nodes.append(node);
}
@@ -300,6 +302,8 @@
case FollowingAxis:
if (context.isAttributeNode()) {
Node* node = static_cast<Attr&>(context).ownerElement();
+ if (!node)
+ return;
while ((node = NodeTraversal::next(*node))) {
if (nodeMatches(*node, FollowingAxis, m_nodeTest))
nodes.append(node);
@@ -319,9 +323,11 @@
return;
case PrecedingAxis: {
Node* node;
- if (context.isAttributeNode())
+ if (context.isAttributeNode()) {
node = static_cast<Attr&>(context).ownerElement();
- else
+ if (!node)
+ return;
+ } else
node = &context;
while (ContainerNode* parent = node->parentNode()) {
for (node = NodeTraversal::previous(*node); node != parent; node = NodeTraversal::previous(*node)) {
@@ -382,6 +388,8 @@
Node* node = &context;
if (context.isAttributeNode()) {
node = static_cast<Attr&>(context).ownerElement();
+ if (!node)
+ return;
if (nodeMatches(*node, AncestorOrSelfAxis, m_nodeTest))
nodes.append(node);
}
