From 90ae142e4c7c5fb94683a6e027f9492d06711abd Mon Sep 17 00:00:00 2001
From: Mason Freed <[email protected]>
Date: Thu, 08 Oct 2026 16:17:34 -0700
Subject: [PATCH] Don't hold a reference to the id attribute across UpdateId()

Element::InsertedInto() took a reference into the element's attribute
storage via GetIdAttribute() and passed it to UpdateId(). UpdateId()
notifies id target observers, and those can update style and layout.
For example, a focused <input list> whose datalist became usable
rebuilds its shadow tree and calls RevealSelection(). If that update
builds a <use> instance tree that clones the element, cloning
serializes the element's dirty style attribute and appends it to the
attribute storage. When that reallocates the storage, the reference
dangles, and the remaining reads of the id in UpdateId() access freed
memory.

This CL copies the id into a local AtomicString before calling
UpdateId(). RemovedFrom() passes the id to UpdateId() the same way, so
it gets the same change. Recent fixes for similar bugs in
ElementRuleCollector and HTMLAnchorElementBase::NavigateToHyperlink()
used the same approach.

Fixed: 567638392
Change-Id: I1ebac072098091e5dc9d8898f2bced3cf0c3bbcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8517119
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: David Baron <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1715019}
---
diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
index 44c9614..93f29f6 100644
--- a/third_party/blink/renderer/core/dom/element.cc
+++ b/third_party/blink/renderer/core/dom/element.cc
@@ -4369,8 +4369,11 @@
     return kInsertionDone;
   }
 
-  const AtomicString& id_value = GetIdAttribute();
+  const AtomicString id_value = GetIdAttribute();
   if (!id_value.IsNull()) {
+    // UpdateId() notifies id observers, which can update style and layout.
+    // That can synchronize the lazy style attribute, which can reallocate the
+    // attribute storage, invalidating references from GetIdAttribute().
     UpdateId(scope, g_null_atom, id_value);
   }
 
@@ -4671,9 +4674,11 @@
 
   SetSavedLayerScrollOffset(ScrollOffset());
 
-  const AtomicString& id_value = GetIdAttribute();
+  const AtomicString id_value = GetIdAttribute();
   if (insertion_point.IsInTreeScope() && GetTreeScope() == document) {
     if (!id_value.IsNull()) {
+      // UpdateId() can invalidate references from GetIdAttribute(). See the
+      // comment in InsertedInto().
       UpdateId(insertion_point.GetTreeScope(), id_value, g_null_atom);
     }
 
diff --git a/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html b/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html
new file mode 100644
index 0000000..77823ae
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:[email protected]">
+<link rel="help" href="https://crbug.com/567638392">
+
+<datalist id="list">
+  <option value="value" disabled></option>
+</datalist>
+<input list="list">
+<svg>
+  <use href="#target"></use>
+</svg>
+
+<script>
+  const input = document.querySelector('input');
+  document.body.offsetTop;
+  input.focus();
+  // This makes the datalist usable, but doesn't notify the input.
+  document.querySelector('option').disabled = false;
+
+  const target = document.createElementNS('http://www.w3.org/2000/svg', 'g');
+  target.id = 'target';
+  const object = document.createElement('object');
+  object.id = 'list';
+  // Fill the attribute storage to (Chromium) capacity, so that adding the
+  // style attribute reallocates it.
+  for (let i = 0; i < 7; ++i) {
+    object.setAttribute(`data-${i}`, '');
+  }
+  object.style.color = 'green';
+  target.appendChild(object);
+  // This test passes if this doesn't crash:
+  document.querySelector('svg').appendChild(target);
+</script>

diff --git a/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html b/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html
new file mode 100644
index 0000000..77823ae
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/html/semantics/forms/the-datalist-element/list-id-insertion-crash.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:[email protected]">
+<link rel="help" href="https://crbug.com/567638392">
+
+<datalist id="list">
+  <option value="value" disabled></option>
+</datalist>
+<input list="list">
+<svg>
+  <use href="#target"></use>
+</svg>
+
+<script>
+  const input = document.querySelector('input');
+  document.body.offsetTop;
+  input.focus();
+  // This makes the datalist usable, but doesn't notify the input.
+  document.querySelector('option').disabled = false;
+
+  const target = document.createElementNS('http://www.w3.org/2000/svg', 'g');
+  target.id = 'target';
+  const object = document.createElement('object');
+  object.id = 'list';
+  // Fill the attribute storage to (Chromium) capacity, so that adding the
+  // style attribute reallocates it.
+  for (let i = 0; i < 7; ++i) {
+    object.setAttribute(`data-${i}`, '');
+  }
+  object.style.color = 'green';
+  target.appendChild(object);
+  // This test passes if this doesn't crash:
+  document.querySelector('svg').appendChild(target);
+</script>