Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate implementation in AdFilter
DescriptionInappropriate implementation in AdFilter
ComponentAdFilter
Bug ClassLogic Error
Tracker501591293
Fix commit6fc209a3988c (chromium/src) +100/-0
CISA KEVNot listed
CreditedGoogle
Disclosed2026-07-29

Changed Functions

FunctionChangeNotes
for
third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
modified
TEST_F
third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
modified
get
third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
modified
if
third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
modified

Files Changed

  • third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
  • third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
From 6fc209a3988cb7d07c23a6583dc4a6ea561bea5d Mon Sep 17 00:00:00 2001
From: Yao Xiao <yaoxia@chromium.org>
Date: Mon, 22 Jun 2026 13:34:08 -0700
Subject: [PATCH] [AdTracker] Prevent script execution during monkey-patch detection

This CL wraps the prototype-chain traversal heuristic in
`AdTracker::GetApiFunctionInfo` with
`v8::Isolate::DisallowJavascriptExecutionScope`.

Rationale:
The previous approach was vulnerable to script execution. Accessing
intermediate objects in the path via `v8::Object::Get()` could
inadvertently trigger author-defined JavaScript getters or proxy traps.
If an attacker used this to execute synchronous JS during restricted
lifecycle phases (like Blink's ScriptForbiddenScope), it would cause
DOM mutation re-entrancy crashes.

By using `DisallowJavascriptExecutionScope`, we guarantee that the
monkey-patch heuristic will not execute script. If an attacker attempts
to shadow an API with an author-defined getter anywhere in the property
path, V8 safely throws an internal exception, the `Get()` call returns
empty, and AdTracker safely aborts the heuristic.

Bug: 501591293
Change-Id: Ia0ab7d0844521cb5a912a4d65cc2459dc611de04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7898065
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1650550}
---

diff --git a/third_party/blink/renderer/core/ad_tracker/ad_tracker.cc b/third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
index b3cc1c9..0623d4a 100644
--- a/third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
+++ b/third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
@@ -85,6 +85,13 @@
   v8::Local<v8::Value> current_value = context->Global();
   const base::span<const char* const> property_path = GetApiPropertyPath(api);
 
+  // Prevent script execution (e.g., via author-defined getters or proxy traps)
+  // during prototype chain traversal to avoid evasion, side effects, or DOM
+  // mutation re-entrancy crashes.
+  v8::Isolate::DisallowJavascriptExecutionScope disallow_js(
+      isolate, v8::Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE);
+  v8::TryCatch try_catch(isolate);
+
   // Traverse the property path (e.g., global object -> `history` ->
   // `pushState`).
   for (const char* property_name : property_path) {
diff --git a/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc b/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
index 93feb60..da7951f 100644
--- a/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
+++ b/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
@@ -4571,4 +4571,97 @@
   EXPECT_TRUE(child_frame->IsFrameCreatedByAdScript());
 }
 
+// Regression test for https://crbug.com/501591293.
+//
+// This test verifies that the AdTracker's monkey-patch heuristic safely aborts
+// and falls back to standard stack-based ad detection if it encounters an
+// author-defined JavaScript getter. This ensures that the tracker evaluates the
+// actual state of the object without handing control over to the script it is
+// trying to evaluate, preventing evasion, side effects, or crashes.
+//
+// This test overrides `window.Node` with a getter that attempts to
+// synchronously mutate the DOM. It asserts that the monkey patch heuristic
+// bails out (defaulting to 'not a monkeypatch') and flags the frame as an ad,
+// without actually evaluating the getter and causing a DOM mutation.
+TEST_F(AdTrackerSimTest, NoScriptExecutionDuringAdTrackerMonkeyPatchCheck) {
+  String ad_script_url = "https://example.com/script.js?ad=true";
+  String vanilla_script_url = "https://example.com/script.js";
+  SimSubresourceRequest ad_script(ad_script_url, "text/javascript");
+  SimSubresourceRequest vanilla_script(vanilla_script_url, "text/javascript");
+
+  main_resource_->Complete(R"HTML(
+    <body><script src="script.js?ad=true"></script>
+          <script src="script.js"></script></body>
+  )HTML");
+
+  // The ad script defines a getter on `window.Node`.
+  // It also monkey-patches appendChild so that the ad script is on the stack
+  // when appendChild is called, triggering the monkey-patch heuristic.
+  ad_script.Complete(R"SCRIPT(
+    window.getterFired = false;
+    let originalNode = window.Node;
+
+    const originalAppendChild = originalNode.prototype.appendChild;
+    originalNode.prototype.appendChild = function(child) {
+      return originalAppendChild.call(this, child);
+    };
+
+    Object.defineProperty(window, 'Node', {
+      configurable: true,
+      get() {
+        console.log("Getter executing!");
+        window.getterFired = true;
+        // Malicious payload: mutate the DOM tree synchronously.
+        // If this runs during AdTracker's inspection (which happens in the
+        // middle of a C++ node insertion loop), it will cause a DOM mutation
+        // re-entrancy and hard-crash the renderer.
+        if (window.iframeElement) {
+          document.body.appendChild(window.iframeElement);
+        }
+        return originalNode;
+      }
+    });
+    )SCRIPT");
+
+  // The vanilla script calls the monkey-patched appendChild.
+  vanilla_script.Complete(R"SCRIPT(
+    let fragment = document.createDocumentFragment();
+    let video = document.createElement("video");
+    window.iframeElement = document.createElement("iframe");
+
+    // 1. The <video> element is necessary because its 'InsertedInto' lifecycle
+    //    triggers the AdTracker to inspect for monkey-patches (evaluating
+    //    window.Node).
+    fragment.appendChild(video);
+
+    // 2. The <iframe> is included in the fragment so that the outer C++
+    //    insertion loop intends to process it *after* the video. If the getter
+    //    above secretly inserts it first, the C++ loop will try to insert it a
+    //    second time, corrupting the tree and crashing the browser.
+    fragment.appendChild(window.iframeElement);
+
+    // Call the monkey-patched appendChild, putting ad script on the stack
+    document.body.appendChild(fragment);
+
+    console.log(window.getterFired ? "Getter Fired" : "Getter Not Fired");
+    )SCRIPT");
+
+  base::RunLoop().RunUntilIdle();
+
+  // The getter should NOT have fired during the check.
+  ASSERT_EQ(1u, ConsoleMessages().size());
+  EXPECT_EQ("Getter Not Fired", ConsoleMessages()[0]);
+
+  // Verify that the iframe is created.
+  auto* child_frame =
+      To<LocalFrame>(GetDocument().GetFrame()->Tree().FirstChild());
+  ASSERT_TRUE(child_frame);
+
+  // Since we bailed out of the monkey-patch check, the exception should be
+  // denied, meaning it falls back to the stack. Since the ad script is on the
+  // stack (via the monkey-patched appendChild), the frame should be correctly
+  // flagged as an ad frame.
+  EXPECT_TRUE(child_frame->IsFrameCreatedByAdScript());
+}
+
 }  // namespace blink
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc b/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
index 93feb60..da7951f 100644
--- a/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
+++ b/third_party/blink/renderer/core/ad_tracker/ad_tracker_test.cc
@@ -4571,4 +4571,97 @@
   EXPECT_TRUE(child_frame->IsFrameCreatedByAdScript());
 }
 
+// Regression test for https://crbug.com/501591293.
+//
+// This test verifies that the AdTracker's monkey-patch heuristic safely aborts
+// and falls back to standard stack-based ad detection if it encounters an
+// author-defined JavaScript getter. This ensures that the tracker evaluates the
+// actual state of the object without handing control over to the script it is
+// trying to evaluate, preventing evasion, side effects, or crashes.
+//
+// This test overrides `window.Node` with a getter that attempts to
+// synchronously mutate the DOM. It asserts that the monkey patch heuristic
+// bails out (defaulting to 'not a monkeypatch') and flags the frame as an ad,
+// without actually evaluating the getter and causing a DOM mutation.
+TEST_F(AdTrackerSimTest, NoScriptExecutionDuringAdTrackerMonkeyPatchCheck) {
+  String ad_script_url = "https://example.com/script.js?ad=true";
+  String vanilla_script_url = "https://example.com/script.js";
+  SimSubresourceRequest ad_script(ad_script_url, "text/javascript");
+  SimSubresourceRequest vanilla_script(vanilla_script_url, "text/javascript");
+
+  main_resource_->Complete(R"HTML(
+    <body><script src="script.js?ad=true"></script>
+          <script src="script.js"></script></body>
+  )HTML");
+
+  // The ad script defines a getter on `window.Node`.
+  // It also monkey-patches appendChild so that the ad script is on the stack
+  // when appendChild is called, triggering the monkey-patch heuristic.
+  ad_script.Complete(R"SCRIPT(
+    window.getterFired = false;
+    let originalNode = window.Node;
+
+    const originalAppendChild = originalNode.prototype.appendChild;
+    originalNode.prototype.appendChild = function(child) {
+      return originalAppendChild.call(this, child);
+    };
+
+    Object.defineProperty(window, 'Node', {
+      configurable: true,
+      get() {
+        console.log("Getter executing!");
+        window.getterFired = true;
+        // Malicious payload: mutate the DOM tree synchronously.
+        // If this runs during AdTracker's inspection (which happens in the
+        // middle of a C++ node insertion loop), it will cause a DOM mutation
+        // re-entrancy and hard-crash the renderer.
+        if (window.iframeElement) {
+          document.body.appendChild(window.iframeElement);
+        }
+        return originalNode;
+      }
+    });
+    )SCRIPT");
+
+  // The vanilla script calls the monkey-patched appendChild.
+  vanilla_script.Complete(R"SCRIPT(
+    let fragment = document.createDocumentFragment();
+    let video = document.createElement("video");
+    window.iframeElement = document.createElement("iframe");
+
+    // 1. The <video> element is necessary because its 'InsertedInto' lifecycle
+    //    triggers the AdTracker to inspect for monkey-patches (evaluating
+    //    window.Node).
+    fragment.appendChild(video);
+
+    // 2. The <iframe> is included in the fragment so that the outer C++
+    //    insertion loop intends to process it *after* the video. If the getter
+    //    above secretly inserts it first, the C++ loop will try to insert it a
+    //    second time, corrupting the tree and crashing the browser.
+    fragment.appendChild(window.iframeElement);
+
+    // Call the monkey-patched appendChild, putting ad script on the stack
+    document.body.appendChild(fragment);
+
+    console.log(window.getterFired ? "Getter Fired" : "Getter Not Fired");
+    )SCRIPT");
+
+  base::RunLoop().RunUntilIdle();
+
+  // The getter should NOT have fired during the check.
+  ASSERT_EQ(1u, ConsoleMessages().size());
+  EXPECT_EQ("Getter Not Fired", ConsoleMessages()[0]);
+
+  // Verify that the iframe is created.
+  auto* child_frame =
+      To<LocalFrame>(GetDocument().GetFrame()->Tree().FirstChild());
+  ASSERT_TRUE(child_frame);
+
+  // Since we bailed out of the monkey-patch check, the exception should be
+  // denied, meaning it falls back to the stack. Since the ad script is on the
+  // stack (via the monkey-patched appendChild), the frame should be correctly
+  // flagged as an ad frame.
+  EXPECT_TRUE(child_frame->IsFrameCreatedByAdScript());
+}
+
 }  // namespace blink
Loading diff…

Original Bug Report

reported by vm...@google.com

ScriptForbiddenScope bypass in AdTracker enables synchronous DOM mutation reentrancy

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team.

Overview: Blink’s AdTracker::GetApiFunctionInfo relies on v8::Object::Get to traverse properties when checking for monkey-patched APIs during DOM insertion. This V8 API intentionally skips the BeforeCallEnteredCallback, bypassing Blink’s ScriptForbiddenScope checks. An attacker can exploit this using a malicious getter to execute synchronous JavaScript and mutate the DOM tree mid-insertion, leading to state corruption and potential renderer RCE.

Affected files:

  • third_party/blink/renderer/core/ad_tracker/ad_tracker.cc
  • third_party/blink/renderer/core/html/media/html_video_element.cc
  • third_party/blink/renderer/core/dom/container_node.cc
  • third_party/blink/renderer/core/html/html_frame_element_base.cc

Estimated timestamp from git blame: 2025-10-28

Background

During DOM tree mutations (e.g., ContainerNode::AppendChild), Blink instantiates a ScriptForbiddenScope and an EventDispatchForbiddenScope to prevent synchronous JavaScript execution. This guarantees that C++ DOM manipulation algorithms and iterators can operate safely without the tree being modified from underneath them.

The Vulnerability

When a <video> element is inserted into the DOM by an ad script, its InsertedInto lifecycle method calls AdTracker::IsAdScriptInStack. To prevent false positives from benign monkey-patching, AdTracker calls GetApiFunctionInfo to verify if standard APIs (like Node.prototype.appendChild) have been tampered with.

GetApiFunctionInfo walks the global object properties using v8::Object::Get(context, property_key). However, v8::Object::Get invokes PrepareForExecutionScope with the template parameter do_callback = false (defined in v8/src/api/api-inl.h).

Because do_callback is false, V8 skips calling isolate_->FireBeforeCallEnteredCallback(). Blink relies entirely on this callback (implemented in V8PerIsolateData::BeforeCallEnteredCallback) to enforce CHECK(!ScriptForbiddenScope::IsScriptForbidden()). Consequently, if an attacker places a JavaScript getter or Proxy on the property path (e.g., window.Node), it will execute synchronously inside V8 while the C++ stack is in the middle of a forbidden scope.

Impact

Bypassing ScriptForbiddenScope during DOM mutation algorithms is a critical violation of Blink’s memory safety invariants. It enables classic DOM mutation reentrancy. An attacker can mutate the tree mid-iteration (e.g., reparenting nodes, creating circular sibling pointers), which leads to Detached Tree Use-After-Free (UAF) and potential Arbitrary Code Execution (RCE) within the sandboxed renderer process.

Potential Reproduction Steps

Note: These steps are generated based on static code analysis by an AI tooling agent and represent a theoretical attack path. A working proof-of-concept has not yet been executed.

  1. An attacker serves a script classified by Blink as an ad script (e.g., via a known ad domain).
  2. The ad script defines a malicious getter on window.Node:
    Object.defineProperty(window, 'Node', {
      configurable: true,
      get() {
        // Executes during ScriptForbiddenScope!
        document.body.appendChild(iframe);
        return originalNode;
      }
    });
    
  3. The ad script constructs a DocumentFragment containing a <video> and the <iframe> element.
  4. The script appends the fragment to the DOM: document.body.appendChild(fragment).
  5. During ContainerNode::InsertNodeVector, the <video> element is processed first. Its InsertedInto method triggers AdTracker, which evaluates window.Node.
  6. The getter fires, synchronously appending the <iframe>. This links the <iframe> into the tree and triggers its InsertedInto lifecycle, initializing its frame.
  7. Control returns to the outer InsertNodeVector loop, which then processes the <iframe> a second time. Because the <iframe> was already appended during the getter, applying the insertion logic again creates a circular sibling pointer (lastChild()->SetNextSibling(&child)) and triggers a SECURITY_CHECK(!ContentFrame()) crash in HTMLFrameElementBase::InsertedInto.

Proposed Fix

There are several ways to address this issue:

  1. Use safe V8 accessors: Refactor AdTracker::GetApiFunctionInfo to avoid triggering author JavaScript. Instead of v8::Object::Get, use v8::Object::GetRealNamedProperty or retrieve the property descriptor to ensure only the native, non-getter value is inspected.
  2. Early exit: Check ScriptForbiddenScope::IsScriptForbidden() explicitly at the top of AdTracker::GetApiFunctionInfo or IsAdScriptInStackHelper, and gracefully return early (e.g., assuming it is not a monkey patch) if script execution is currently forbidden.

Evaluated with Chrome root at commit: 096fc8fdbfacf2546485756d03f160a3d04fcc9b


Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker