From 6c393f613a638b6c090fcf01368b3482861732bd Mon Sep 17 00:00:00 2001
From: Dominic Battre <[email protected]>
Date: Thu, 25 Jun 2026 09:01:12 -0700
Subject: [PATCH] [DOM] Skip in-use DOMNodeIds when the id counter wraps

DOMNodeIds::IdForNode() assigns ids from a process-wide 32-bit counter.
When it wraps it inserted into the id->Node map with HeapHashMap::Set(),
which overwrites an existing entry, so a still-live Node could have its
id reassigned to a new Node and NodeForId() would resolve to the wrong
one.

Mirror WeakIdentifierMap and retry with the next id while the candidate
is already present in the map. Factor the counter into NextId()/LastId()
helpers and add SetLastIdForTesting() so the wrap path can be exercised
from a unit test.

Fixed: 501675996
Change-Id: I33d958b088535850109135076ab7a1056a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7977997
Commit-Queue: Dominic Battré <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Auto-Submit: Dominic Battré <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1652464}
---

diff --git a/third_party/blink/renderer/core/dom/dom_node_ids.cc b/third_party/blink/renderer/core/dom/dom_node_ids.cc
index 09cebb84..57611bb 100644
--- a/third_party/blink/renderer/core/dom/dom_node_ids.cc
+++ b/third_party/blink/renderer/core/dom/dom_node_ids.cc
@@ -13,13 +13,27 @@
 
 namespace blink {
 
-static GCedHeapHashMap<DOMNodeId, WeakMember<Node>>& IdToNodeMap() {
+namespace {
+
+DOMNodeId g_last_id = 0;
+
+// See WeakIdentifierMap::Next().
+DOMNodeId NextId() {
+  if (g_last_id == std::numeric_limits<DOMNodeId>::max()) [[unlikely]] {
+    g_last_id = 0;
+  }
+  return ++g_last_id;
+}
+
+GCedHeapHashMap<DOMNodeId, WeakMember<Node>>& IdToNodeMap() {
   using RefType = GCedHeapHashMap<DOMNodeId, WeakMember<Node>>;
   DEFINE_STATIC_LOCAL(Persistent<RefType>, map_instance,
                       (MakeGarbageCollected<RefType>()));
   return *map_instance;
 }
 
+}  // namespace
+
 // static
 DOMNodeId DOMNodeIds::ExistingIdForNode(Node* node) {
   return node ? node->NodeID(base::PassKey<DOMNodeIds>()) : kInvalidDOMNodeId;
@@ -38,18 +52,20 @@
 
   DOMNodeId& id = node->EnsureNodeID(base::PassKey<DOMNodeIds>());
   if (id == kInvalidDOMNodeId) {
-    // See WeakIdentifierMap::Next().
-    static DOMNodeId last_id = 0;
-    if (last_id == std::numeric_limits<DOMNodeId>::max()) [[unlikely]] {
-      last_id = 0;
+    id = NextId();
+    while (!IdToNodeMap().insert(id, node).is_new_entry) [[unlikely]] {
+      id = NextId();
     }
-    id = ++last_id;
-    IdToNodeMap().Set(id, node);
   }
   return id;
 }
 
 // static
+void DOMNodeIds::SetLastIdForTesting(DOMNodeId id) {
+  g_last_id = id;
+}
+
+// static
 Node* DOMNodeIds::NodeForId(DOMNodeId id) {
   if (id == kInvalidDOMNodeId) {
     return nullptr;
diff --git a/third_party/blink/renderer/core/dom/dom_node_ids.h b/third_party/blink/renderer/core/dom/dom_node_ids.h
index fa209597..eaefff22 100644
--- a/third_party/blink/renderer/core/dom/dom_node_ids.h
+++ b/third_party/blink/renderer/core/dom/dom_node_ids.h
@@ -28,6 +28,8 @@
 
   // Return a node for the DOMNodeID or null if one hasn't been assigned.
   static Node* NodeForId(DOMNodeId);
+
+  static void SetLastIdForTesting(DOMNodeId);
 };
 
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/dom/dom_node_ids_test.cc b/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
index 04425ec..c9e27de 100644
--- a/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
+++ b/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
@@ -58,6 +58,24 @@
   EXPECT_EQ(nullptr, DOMNodeIds::NodeForId(kInvalidDOMNodeId));
 }
 
+TEST_F(DOMNodeIdsTest, Overflow) {
+  SetBodyContent("<div id='a'></div><div id='b'></div>");
+  Node* a = GetDocument().getElementById(AtomicString("a"));
+  Node* b = GetDocument().getElementById(AtomicString("b"));
+
+  DOMNodeId id_a = a->GetDomNodeId();
+  EXPECT_EQ(a, DOMNodeIds::NodeForId(id_a));
+
+  // Simulate the counter having wrapped around back to just before |id_a|. The
+  // next assigned id must skip over |id_a| while |a| is still alive.
+  DOMNodeIds::SetLastIdForTesting(id_a - 1);
+
+  DOMNodeId id_b = b->GetDomNodeId();
+  EXPECT_NE(id_a, id_b);
+  EXPECT_EQ(a, DOMNodeIds::NodeForId(id_a));
+  EXPECT_EQ(b, DOMNodeIds::NodeForId(id_b));
+}
+
 TEST_F(DOMNodeIdsTest, ExistingIdForNode) {
   SetBodyContent("<div id='a'></div>");
   Node* a = GetDocument().getElementById(AtomicString("a"));

diff --git a/third_party/blink/renderer/core/dom/dom_node_ids_test.cc b/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
index 04425ec..c9e27de 100644
--- a/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
+++ b/third_party/blink/renderer/core/dom/dom_node_ids_test.cc
@@ -58,6 +58,24 @@
   EXPECT_EQ(nullptr, DOMNodeIds::NodeForId(kInvalidDOMNodeId));
 }
 
+TEST_F(DOMNodeIdsTest, Overflow) {
+  SetBodyContent("<div id='a'></div><div id='b'></div>");
+  Node* a = GetDocument().getElementById(AtomicString("a"));
+  Node* b = GetDocument().getElementById(AtomicString("b"));
+
+  DOMNodeId id_a = a->GetDomNodeId();
+  EXPECT_EQ(a, DOMNodeIds::NodeForId(id_a));
+
+  // Simulate the counter having wrapped around back to just before |id_a|. The
+  // next assigned id must skip over |id_a| while |a| is still alive.
+  DOMNodeIds::SetLastIdForTesting(id_a - 1);
+
+  DOMNodeId id_b = b->GetDomNodeId();
+  EXPECT_NE(id_a, id_b);
+  EXPECT_EQ(a, DOMNodeIds::NodeForId(id_a));
+  EXPECT_EQ(b, DOMNodeIds::NodeForId(id_b));
+}
+
 TEST_F(DOMNodeIdsTest, ExistingIdForNode) {
   SetBodyContent("<div id='a'></div>");
   Node* a = GetDocument().getElementById(AtomicString("a"));