From 7bf572e7a2ed8c0d893e686b232f018d36bcd6cb Mon Sep 17 00:00:00 2001
From: Benjamin Beaudry <[email protected]>
Date: Thu, 16 Apr 2026 17:11:41 -0700
Subject: [PATCH] [a11y] Guard against node destruction during alert event dispatch

This CL fixes a potential use-after-free in AXPlatformNodeWin where a
node destroyed during the reentrant message pump inside an alert event
notification would be inserted into the process-global alert targets
set as a dangling pointer. See bug for details.

We fix it by preventing AddAlertTarget() from inserting a node that is
already destroyed, and add a regression test that reproduces the
post-reentrancy state and asserts the global set is unchanged.

Fixed: 503419515
Change-Id: I6721a22bd0ba67b0d6d4c8ef84d7cabd5447d6d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7769271
Auto-Submit: Benjamin Beaudry <[email protected]>
Commit-Queue: Benjamin Beaudry <[email protected]>
Reviewed-by: Kevin Babbitt <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1616207}
---

diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc
index a5354a2..5832649 100644
--- a/ui/accessibility/platform/ax_platform_node_win.cc
+++ b/ui/accessibility/platform/ax_platform_node_win.cc
@@ -8315,9 +8315,23 @@
 }
 
 void AXPlatformNodeWin::AddAlertTarget() {
+  // Firing an alert event can reentrantly destroy this node via the STA
+  // message pump; don't insert a dangling pointer. crbug.com/503419515.
+  if (IsDestroyed()) {
+    return;
+  }
   GetAlertTargets().insert(this);
 }
 
+// static
+size_t AXPlatformNodeWin::GetAlertTargetCountForTesting() {
+  return GetAlertTargets().size();
+}
+
+void AXPlatformNodeWin::AddAlertTargetForTesting() {
+  AddAlertTarget();
+}
+
 void AXPlatformNodeWin::RemoveAlertTarget() {
   if (GetAlertTargets().find(this) != GetAlertTargets().end()) {
     GetAlertTargets().erase(this);
diff --git a/ui/accessibility/platform/ax_platform_node_win.h b/ui/accessibility/platform/ax_platform_node_win.h
index 7413fc3..1101800 100644
--- a/ui/accessibility/platform/ax_platform_node_win.h
+++ b/ui/accessibility/platform/ax_platform_node_win.h
@@ -1231,6 +1231,12 @@
   // see above.
   static Counts ResetCountsForTesting();
 
+  // Returns the size of the process-global alert targets set.
+  static size_t GetAlertTargetCountForTesting();
+
+  // Test-only wrapper around AddAlertTarget().
+  void AddAlertTargetForTesting();
+
   bool IsUIAControl() const;
 
  protected:
diff --git a/ui/accessibility/platform/ax_platform_node_win_unittest.cc b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
index a34792d..9bb2610b 100644
--- a/ui/accessibility/platform/ax_platform_node_win_unittest.cc
+++ b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
@@ -8291,4 +8291,39 @@
   }
 }
 
+// Regression test for crbug.com/503419515: a node destroyed mid-event must
+// not be inserted into the global alert targets set.
+TEST_F(AXPlatformNodeWinTest, DestroyedNodeNotAddedToAlertTargets) {
+  AXNodeData root;
+  root.id = 1;
+  root.role = ax::mojom::Role::kRootWebArea;
+  root.child_ids = {2};
+
+  AXNodeData alert;
+  alert.id = 2;
+  alert.role = ax::mojom::Role::kAlert;
+
+  Init(root, alert);
+  AXNode* alert_ax_node = GetRoot()->children()[0];
+
+  auto* alert_node = static_cast<AXPlatformNodeWin*>(
+      AXPlatformNodeFromNode(alert_ax_node));
+  ASSERT_TRUE(alert_node);
+
+  const size_t initial_count =
+      AXPlatformNodeWin::GetAlertTargetCountForTesting();
+
+  // Put the node in the IsDestroyed() state without actually destroying it,
+  // so the wrapper can still tear down cleanly at the end of the test.
+  AXPlatformNodeDelegate* original_delegate =
+      alert_node->SetDelegateForTesting(nullptr);
+  ASSERT_TRUE(alert_node->IsDestroyed());
+
+  alert_node->AddAlertTargetForTesting();
+  EXPECT_EQ(initial_count,
+            AXPlatformNodeWin::GetAlertTargetCountForTesting());
+
+  alert_node->SetDelegateForTesting(original_delegate);
+}
+
 }  // namespace ui

diff --git a/ui/accessibility/platform/ax_platform_node_win_unittest.cc b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
index a34792d..9bb2610b 100644
--- a/ui/accessibility/platform/ax_platform_node_win_unittest.cc
+++ b/ui/accessibility/platform/ax_platform_node_win_unittest.cc
@@ -8291,4 +8291,39 @@
   }
 }
 
+// Regression test for crbug.com/503419515: a node destroyed mid-event must
+// not be inserted into the global alert targets set.
+TEST_F(AXPlatformNodeWinTest, DestroyedNodeNotAddedToAlertTargets) {
+  AXNodeData root;
+  root.id = 1;
+  root.role = ax::mojom::Role::kRootWebArea;
+  root.child_ids = {2};
+
+  AXNodeData alert;
+  alert.id = 2;
+  alert.role = ax::mojom::Role::kAlert;
+
+  Init(root, alert);
+  AXNode* alert_ax_node = GetRoot()->children()[0];
+
+  auto* alert_node = static_cast<AXPlatformNodeWin*>(
+      AXPlatformNodeFromNode(alert_ax_node));
+  ASSERT_TRUE(alert_node);
+
+  const size_t initial_count =
+      AXPlatformNodeWin::GetAlertTargetCountForTesting();
+
+  // Put the node in the IsDestroyed() state without actually destroying it,
+  // so the wrapper can still tear down cleanly at the end of the test.
+  AXPlatformNodeDelegate* original_delegate =
+      alert_node->SetDelegateForTesting(nullptr);
+  ASSERT_TRUE(alert_node->IsDestroyed());
+
+  alert_node->AddAlertTargetForTesting();
+  EXPECT_EQ(initial_count,
+            AXPlatformNodeWin::GetAlertTargetCountForTesting());
+
+  alert_node->SetDelegateForTesting(original_delegate);
+}
+
 }  // namespace ui