Infinite recursion caused by call to accessibilityIsIgnored in the mi…

…dst of AccessibilityObject::ignoredFromModalPresence https://bugs.webkit.org/show_bug.cgi?id=240365 Reviewed by Chris Fleizach. Source/WebCore: We can get infinite recursion when accessibilityIsIgnored is called as part of computing AccessibilityObject::ignoredFromModalPresence. One example of such a cycle: AXObjectCache::currentModalNode() -> AccessibilityRenderObject::computeAccessibilityIsIgnored() -> AccessibilityRenderObject::parentObjectUnignored() -> AccessibilityObject::accessibilityIsIgnored() -> AccessibilityObject::ignoredFromModalPresence() -> AXObjectCache::currentModalNode() -> ...repeat... This patch fixes this by tracking when we start computing the current modal node in the AXObjectCache. Then, in AccessibilityObject::accessibilityIsIgnored(), we don't call AccessibilityObject::ignoredFromModalPresence() if this new state is true, since in this context we only need to know if the object is inherently ignored (i.e. ignored disregarding modal presence). Test: accessibility/aria-modal-with-text-crash.html * accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::currentModalNode): * accessibility/AXObjectCache.h: Add m_isRetrievingCurrentModalNode. (WebCore::AXObjectCache::isRetrievingCurrentModalNode): Added. * accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::accessibilityIsIgnored const): Don't call ignoredFromModalPresence if we're in the midst of computing the current modal. LayoutTests: * accessibility/aria-modal-with-text-crash-expected.txt: Added. * accessibility/aria-modal-with-text-crash.html: Added. * platform/glib/TestExpectations: Skip new test. * platform/ios/TestExpectations: Enable new test. * platform/win/TestExpectations: Skip new test. Canonical link: https://commits.webkit.org/250552@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294186 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 14, 2022 · 3d934cd81b30c00dc18ddee2aef8d760dd1db72a · 3d934cd
1 parent eae6342
commit 3d934cd81b30c00dc18ddee2aef8d760dd1db72a
Showing 10 changed files with 107 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2022-05-13  Tyler Wilcock  <tyler_w@apple.com>
+
+        Infinite recursion caused by call to accessibilityIsIgnored in the midst of AccessibilityObject::ignoredFromModalPresence
+        https://bugs.webkit.org/show_bug.cgi?id=240365
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/aria-modal-with-text-crash-expected.txt: Added.
+        * accessibility/aria-modal-with-text-crash.html: Added.
+        * platform/glib/TestExpectations: Skip new test.
+        * platform/ios/TestExpectations: Enable new test.
+        * platform/win/TestExpectations: Skip new test.
+
 2022-05-13  Tim Nguyen  <ntim@apple.com>
 
         [css-ui] Remove caret/progress-bar-value/listitem values from appearance property

diff --git a/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt b/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt
@@ -0,0 +1,10 @@
+This test ensures we don't crash when using search to traverse an aria-modal with text.
+
+
+AXRole: AXStaticText
+AXValue: Foo
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Foo
diff --git a/LayoutTests/accessibility/aria-modal-with-text-crash.html b/LayoutTests/accessibility/aria-modal-with-text-crash.html
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
+</head>
+<body>
+
+<div id="modal" role="dialog" aria-modal="true">
+  <span>Foo</span>
+</div>
+
+<script>
+    var testOutput = "This test ensures we don't crash when using search to traverse an aria-modal with text.\n\n";
+
+    if (window.accessibilityController) {
+        const modal = accessibilityController.accessibleElementById("modal");
+        let searchResult = null;
+        while (true) {
+            searchResult = modal.uiElementForSearchPredicate(searchResult, true, "AXAnyTypeSearchKey", "", false);
+            if (!searchResult)
+                break;
+            const role = searchResult.role;
+            testOutput += `\n${role}`;
+            if (role.includes("StaticText")) {
+                let textContent = accessibilityController.platformName === "ios" ? searchResult.description : searchResult.stringValue;
+                testOutput += `\n${textContent}`;
+            }
+            testOutput += "\n";
+        }
+        debug(testOutput);
+    }
+</script>
+</body>
+</html>
+
+
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -351,6 +351,7 @@ accessibility/aria-busy-updates-after-dynamic-change.html [ Skip ]
 accessibility/text-updates-after-dynamic-change.html [ Skip ]
 
 # Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
+accessibility/aria-modal-with-text-crash.html [ Skip ]
 accessibility/display-contents-search-traversal.html [ Skip ]
 accessibility/search-traversal-after-role-change.html [ Skip ]
 accessibility/table-search-traversal.html [ Skip ]

diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -2135,6 +2135,7 @@ accessibility/aria-sort-changed-notification.html [ Pass ]
 # Enable "aria-table-attributes" test for iOS
 webkit.org/b/150366 accessibility/aria-table-attributes.html [ Pass ]
 
+accessibility/aria-modal-with-text-crash.html [ Pass ]
 accessibility/display-contents-search-traversal.html [ Pass ]
 accessibility/search-traversal-after-role-change.html [ Pass ]
 accessibility/table-search-traversal.html [ Pass ]

diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -481,6 +481,7 @@ accessibility/visible-character-range-width-changes.html [ Skip ]
 accessibility/ancestor-computation.html [ Skip ]
 
 # Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
+accessibility/aria-modal-with-text-crash.html [ Skip ]
 accessibility/display-contents-search-traversal.html [ Skip ]
 accessibility/search-traversal-after-role-change.html [ Skip ]
 accessibility/table-search-traversal.html [ Skip ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2022-05-13  Tyler Wilcock  <tyler_w@apple.com>
+
+        Infinite recursion caused by call to accessibilityIsIgnored in the midst of AccessibilityObject::ignoredFromModalPresence
+        https://bugs.webkit.org/show_bug.cgi?id=240365
+
+        Reviewed by Chris Fleizach.
+
+        We can get infinite recursion when accessibilityIsIgnored is called as
+        part of computing AccessibilityObject::ignoredFromModalPresence. One
+        example of such a cycle:
+
+        AXObjectCache::currentModalNode() ->
+        AccessibilityRenderObject::computeAccessibilityIsIgnored() ->
+        AccessibilityRenderObject::parentObjectUnignored() ->
+        AccessibilityObject::accessibilityIsIgnored() ->
+        AccessibilityObject::ignoredFromModalPresence() ->
+        AXObjectCache::currentModalNode() ->
+        ...repeat...
+
+        This patch fixes this by tracking when we start computing the current
+        modal node in the AXObjectCache. Then, in AccessibilityObject::accessibilityIsIgnored(),
+        we don't call AccessibilityObject::ignoredFromModalPresence() if this new state is true,
+        since in this context we only need to know if the object is inherently
+        ignored (i.e. ignored disregarding modal presence).
+
+        Test: accessibility/aria-modal-with-text-crash.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::currentModalNode):
+        * accessibility/AXObjectCache.h:
+        Add m_isRetrievingCurrentModalNode.
+        (WebCore::AXObjectCache::isRetrievingCurrentModalNode): Added.
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::accessibilityIsIgnored const):
+        Don't call ignoredFromModalPresence if we're in the midst of computing the current modal.
+
 2022-05-13  Brent Fulgham  <bfulgham@apple.com>
 
         REGRESSION (r281791): [iOS] WKWebView cannot load local .log file

diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -306,6 +306,7 @@ Element* AXObjectCache::currentModalNode()
         return activeModalDialog;
     }
 
+    SetForScope retrievingCurrentModalNode(m_isRetrievingCurrentModalNode, true);
     // If any of the modal nodes contains the keyboard focus, we want to pick that one.
     // If not, we want to pick the last visible dialog in the DOM.
     RefPtr<Element> focusedElement = document().focusedElement();

diff --git a/Source/WebCore/accessibility/AXObjectCache.h b/Source/WebCore/accessibility/AXObjectCache.h
@@ -199,7 +199,8 @@ class AXObjectCache {
     void deferNodeAddedOrRemoved(Node*);
     void handleScrolledToAnchor(const Node* anchorNode);
     void handleScrollbarUpdate(ScrollView*);
-
+
+    bool isRetrievingCurrentModalNode() { return m_isRetrievingCurrentModalNode; }
     Node* modalNode();
 
     void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
@@ -518,6 +519,7 @@ class AXObjectCache {
     // If that changes to require only one aria-modal we could change this to a WeakHashSet, or discard the set completely.
     ListHashSet<Element*> m_modalElementsSet;
     bool m_modalNodesInitialized { false };
+    bool m_isRetrievingCurrentModalNode { false };
 
     Timer m_performCacheUpdateTimer;
 

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -3746,7 +3746,10 @@ bool AccessibilityObject::accessibilityIsIgnored() const
         }
     }
 
-    bool ignored = ignoredFromModalPresence();
+    // If we are in the midst of retrieving the current modal node, we only need to consider whether the object
+    // is inherently ignored via computeAccessibilityIsIgnored. Also, calling ignoredFromModalPresence
+    // in this state would cause infinite recursion.
+    bool ignored = cache && cache->isRetrievingCurrentModalNode() ? false : ignoredFromModalPresence();
     if (!ignored)
         ignored = computeAccessibilityIsIgnored();