From 65fba3dbd962b7c36cc657c48ce7705e76ba38c9 Mon Sep 17 00:00:00 2001
From: Stephen Nusko <[email protected]>
Date: Thu, 23 Apr 2026 21:26:52 -0700
Subject: [PATCH] Validate AXNodeID from the renderer.

Currently the renderer can send any value as an AXNodeID, but some are
reserved as invalid or browser specific. This prevents the renderer from
setting them thus causing a collision.

This is phase 1 fix (internal-only): go/project-fortify-review-explainer

Fuchsia-Binary-Size: Size increase is just includes for security fix
Bug: 498205735
Change-Id: I1b62bc2fe2eaa0e96cc44e11c48790c15c14cb88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7727677
Reviewed-by: Hidehiko Abe <[email protected]>
Commit-Queue: Stephen Nusko <[email protected]>
Reviewed-by: David Tseng <[email protected]>
Auto-Submit: Stephen Nusko <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1619955}
---

diff --git a/ui/accessibility/ax_node_id_forward.h b/ui/accessibility/ax_node_id_forward.h
index f5a3e85..6fcd1b5 100644
--- a/ui/accessibility/ax_node_id_forward.h
+++ b/ui/accessibility/ax_node_id_forward.h
@@ -35,6 +35,12 @@
 static constexpr AXNodeID kFirstGeneratedRendererNodeID = -1000000001;
 static constexpr AXNodeID kLastGeneratedRendererNodeID = INT_MIN;
 
+// Validation for AXNodeID from a renderer.
+// Browser reserves [-1,000,000,000, -1] for internal nodes.
+constexpr bool IsValidAXNodeIDFromRenderer(int32_t id) {
+  return id >= kInvalidAXNodeID || id <= ui::kFirstGeneratedRendererNodeID;
+}
+
 }  // namespace ui
 
 #endif  // UI_ACCESSIBILITY_AX_NODE_ID_FORWARD_H_
diff --git a/ui/accessibility/mojom/ax_node_data_mojom_traits.cc b/ui/accessibility/mojom/ax_node_data_mojom_traits.cc
index 7414fa88..61b76450 100644
--- a/ui/accessibility/mojom/ax_node_data_mojom_traits.cc
+++ b/ui/accessibility/mojom/ax_node_data_mojom_traits.cc
@@ -5,6 +5,7 @@
 #include "ui/accessibility/mojom/ax_node_data_mojom_traits.h"
 
 #include "base/containers/flat_map.h"
+#include "ui/accessibility/ax_node_id_forward.h"
 #include "ui/accessibility/mojom/ax_relative_bounds.mojom-shared.h"
 #include "ui/accessibility/mojom/ax_relative_bounds_mojom_traits.h"
 
@@ -34,6 +35,9 @@
 bool StructTraits<ax::mojom::AXNodeDataDataView, ui::AXNodeData>::Read(
     ax::mojom::AXNodeDataDataView data,
     ui::AXNodeData* out) {
+  if (!ui::IsValidAXNodeIDFromRenderer(data.id())) {
+    return false;
+  }
   out->id = data.id();
   out->role = data.role();
   out->state = ui::AXStates(data.state());
@@ -108,8 +112,16 @@
     return false;
   out->html_attributes = std::move(html_attributes).extract();
 
-  if (!data.ReadChildIds(&out->child_ids))
+  std::vector<int32_t> child_ids;
+  if (!data.ReadChildIds(&child_ids)) {
     return false;
+  }
+  for (int32_t child_id : child_ids) {
+    if (!ui::IsValidAXNodeIDFromRenderer(child_id)) {
+      return false;
+    }
+  }
+  out->child_ids = std::move(child_ids);
 
   if (!data.ReadRelativeBounds(&out->relative_bounds))
     return false;
diff --git a/ui/accessibility/mojom/ax_tree_update_mojom_traits.cc b/ui/accessibility/mojom/ax_tree_update_mojom_traits.cc
index d8bfe823..59672c8 100644
--- a/ui/accessibility/mojom/ax_tree_update_mojom_traits.cc
+++ b/ui/accessibility/mojom/ax_tree_update_mojom_traits.cc
@@ -4,6 +4,8 @@
 
 #include "ui/accessibility/mojom/ax_tree_update_mojom_traits.h"
 
+#include "ui/accessibility/ax_node_id_forward.h"
+
 namespace mojo {
 
 // static
@@ -13,7 +15,13 @@
   out->has_tree_data = data.has_tree_data();
   if (!data.ReadTreeData(&out->tree_data))
     return false;
+  if (!ui::IsValidAXNodeIDFromRenderer(data.node_id_to_clear())) {
+    return false;
+  }
   out->node_id_to_clear = data.node_id_to_clear();
+  if (!ui::IsValidAXNodeIDFromRenderer(data.root_id())) {
+    return false;
+  }
   out->root_id = data.root_id();
   if (!data.ReadNodes(&out->nodes))
     return false;
diff --git a/ui/accessibility/platform/browser_accessibility.h b/ui/accessibility/platform/browser_accessibility.h
index 6d674e2..7aba93b3 100644
--- a/ui/accessibility/platform/browser_accessibility.h
+++ b/ui/accessibility/platform/browser_accessibility.h
@@ -15,6 +15,7 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/memory/advanced_memory_safety_checks.h"
 #include "base/memory/raw_ptr.h"
 #include "build/build_config.h"
 #include "ui/accessibility/ax_enums.mojom-forward.h"
@@ -42,6 +43,9 @@
 // Web.
 class COMPONENT_EXPORT(AX_PLATFORM) BrowserAccessibility
     : public AXPlatformNodeDelegate {
+  // TODO(b/498205735): Remove once hardening protections are no longer needed.
+  ADVANCED_MEMORY_SAFETY_CHECKS();
+
  public:
   // Creates a platform specific BrowserAccessibility. Ownership passes to the
   // caller.
diff --git a/ui/accessibility/platform/browser_accessibility_manager.cc b/ui/accessibility/platform/browser_accessibility_manager.cc
index d7e4cb1..f5bcdc43 100644
--- a/ui/accessibility/platform/browser_accessibility_manager.cc
+++ b/ui/accessibility/platform/browser_accessibility_manager.cc
@@ -268,7 +268,9 @@
     return;
   }
 
-  auto* wrapper = GetFromAXNode(node);
+  // Mitigation for b/498205735. Using raw_ptr on the stack as defense-in-depth
+  // to ensure memory is quarantined if freed during the loop below.
+  raw_ptr<BrowserAccessibility> wrapper = GetFromAXNode(node);
   DCHECK(wrapper);
 
   const auto& node_data = wrapper->GetData();