From 5b7fd82e6c332f92840e8762cdaf218f009ddc3c Mon Sep 17 00:00:00 2001
From: Yoshisto Yanagisawa <yyanagisawa@chromium.org>
Date: Fri, 10 Apr 2026 05:59:37 -0700
Subject: [PATCH] Prevent double initialization and Release() in DedicatedWorkerDevToolsAgentHost

A compromised renderer could trigger multiple ChildTargetCreated calls
for the same dedicated worker via a shared worker's DevTools interface.
Each call could establish a new mojo pipe, leading to multiple
SetRenderer() calls and subsequent multiple Release() calls on the host
object when the pipes were closed. This could underflow the reference
count and cause a Use-After-Free (UAF) in the browser process.

This CL fixes the issue in two ways:
1. It prevents multiple initializations of a
   DedicatedWorkerDevToolsAgentHost in
   DevToolsRendererChannel::ChildTargetCreated(). If the agent host
   already has a renderer set, further requests for the same token are
   ignored.
2. It introduces a flag (kWorkerOrWorkletAgentDoubleReleaseFix)
   to ensure that the self-reference (AddRef() in constructor) is
   only released once during Disconnected() in the base class
   WorkerOrWorkletDevToolsAgentHost.

Additionally, a DCHECK in ChildTargetCreated is promoted to CHECK to
ensure that the token must correspond to an existing agent host.

The refcount fix is gated behind a feature flag to serve as a kill
switch for easier merging into older release branches.

Bug: 500136078, 493652473
Change-Id: Ib1a58b9a7f3f63f0362d40476f290645ee865c88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7736861
Auto-Submit: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1612797}
---

diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
index 45c44d0..77067be7 100644
--- a/content/browser/bad_message.h
+++ b/content/browser/bad_message.h
@@ -364,6 +364,7 @@
   RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME = 336,
   RFH_OPEN_URL_INVALID_DISPOSITION = 337,
   RFH_ENTER_FULLSCREEN_PERMISSION_DENIED = 338,
+  DT_DUPLICATE_CHILD_TARGET_CREATED = 339,
 
   // Please add new elements here. The naming convention is abbreviated class
   // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
diff --git a/content/browser/devtools/dedicated_worker_devtools_agent_host.h b/content/browser/devtools/dedicated_worker_devtools_agent_host.h
index 314625a..eab2a740 100644
--- a/content/browser/devtools/dedicated_worker_devtools_agent_host.h
+++ b/content/browser/devtools/dedicated_worker_devtools_agent_host.h
@@ -48,6 +48,8 @@
       const std::string& name,
       base::OnceCallback<void(DevToolsAgentHostImpl*)> callback);
 
+  bool child_worker_created() const { return child_worker_created_; }
+
  private:
   ~DedicatedWorkerDevToolsAgentHost() override;
 
diff --git a/content/browser/devtools/devtools_refcount_unittest.cc b/content/browser/devtools/devtools_refcount_unittest.cc
new file mode 100644
index 0000000..05da47f
--- /dev/null
+++ b/content/browser/devtools/devtools_refcount_unittest.cc
@@ -0,0 +1,93 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/bind.h"
+#include "content/browser/devtools/worker_or_worklet_devtools_agent_host.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/mock_render_process_host.h"
+#include "content/public/test/test_browser_context.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+class TestAgentHost : public WorkerOrWorkletDevToolsAgentHost {
+ public:
+  TestAgentHost(
+      int process_id,
+      const GURL& url,
+      const std::string& name,
+      const base::UnguessableToken& devtools_worker_token,
+      const std::string& parent_id,
+      base::OnceCallback<void(DevToolsAgentHostImpl*)> destroyed_callback)
+      : WorkerOrWorkletDevToolsAgentHost(process_id,
+                                         url,
+                                         name,
+                                         devtools_worker_token,
+                                         parent_id,
+                                         std::move(destroyed_callback)) {
+    NotifyCreated();
+  }
+
+  using WorkerOrWorkletDevToolsAgentHost::Disconnected;
+
+  // DevToolsAgentHost implementation:
+  std::string GetType() override { return "test"; }
+  bool AttachSession(DevToolsSession* session) override { return true; }
+  void DetachSession(DevToolsSession* session) override {}
+
+ private:
+  ~TestAgentHost() override = default;
+};
+
+}  // namespace
+
+class DevToolsRefcountTest : public testing::Test {
+ public:
+  DevToolsRefcountTest()
+      : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP) {}
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestBrowserContext browser_context_;
+};
+
+// Tests that WorkerOrWorkletDevToolsAgentHost doesn't underflow refcount
+// when Disconnected() is called multiple times.
+TEST_F(DevToolsRefcountTest, PreventDoubleRelease) {
+  MockRenderProcessHost renderer_host(&browser_context_);
+  const int process_id = renderer_host.GetDeprecatedID();
+  base::UnguessableToken token = base::UnguessableToken::Create();
+
+  bool destroyed_called = false;
+  auto destroyed_callback = base::BindLambdaForTesting(
+      [&](DevToolsAgentHostImpl* host) { destroyed_called = true; });
+
+  // 1. Create a TestAgentHost.
+  // It starts with refcount 1 (from our scoped_refptr) + 1 (from
+  // self-keep-alive in constructor).
+  scoped_refptr<TestAgentHost> agent_host = base::MakeRefCounted<TestAgentHost>(
+      process_id, GURL("http://example.com"), "worker", token, "parent_id",
+      std::move(destroyed_callback));
+
+  // 2. Call Disconnected() first time.
+  // This should trigger the first Release() and destroyed_callback.
+  agent_host->Disconnected();
+  EXPECT_TRUE(destroyed_called);
+
+  // 3. Call Disconnected() multiple times.
+  // If the fix is working, these subsequent calls won't cause double-release.
+  // We reset destroyed_called to ensure it's not called again.
+  destroyed_called = false;
+  agent_host->Disconnected();
+  agent_host->Disconnected();
+  EXPECT_FALSE(destroyed_called);
+
+  // 4. Verification: If we can still access agent_host, it hasn't been
+  // over-released. The refcount should be 1 (only our scoped_refptr).
+  EXPECT_TRUE(agent_host->HasOneRef());
+}
+
+}  // namespace content
diff --git a/content/browser/devtools/devtools_renderer_channel.cc b/content/browser/devtools/devtools_renderer_channel.cc
index 466d7fc..62413dc7 100644
--- a/content/browser/devtools/devtools_renderer_channel.cc
+++ b/content/browser/devtools/devtools_renderer_channel.cc
@@ -7,6 +7,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/memory/safety_checks.h"
+#include "content/browser/bad_message.h"
 #include "content/browser/devtools/dedicated_worker_devtools_agent_host.h"
 #include "content/browser/devtools/devtools_agent_host_impl.h"
 #include "content/browser/devtools/devtools_manager.h"
@@ -14,6 +15,7 @@
 #include "content/browser/devtools/protocol/devtools_domain_handler.h"
 #include "content/browser/devtools/worker_devtools_manager.h"
 #include "content/browser/devtools/worklet_devtools_agent_host.h"
+#include "content/common/features.h"
 #include "content/public/browser/child_process_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
@@ -185,7 +187,7 @@
     case blink::mojom::DevToolsExecutionContextType::kDedicatedWorker: {
       // WorkerDevToolsAgentHost for dedicated workers is already created in the
       // browser process.
-      DCHECK(content::DevToolsAgentHost::GetForId(
+      CHECK(content::DevToolsAgentHost::GetForId(
           devtools_worker_token.ToString()));
       DedicatedWorkerDevToolsAgentHost* dedicated_worker_agent_host =
           WorkerDevToolsManager::GetInstance().GetDevToolsHostFromToken(
@@ -197,6 +199,15 @@
         // case.
         return;
       }
+      if (base::FeatureList::IsEnabled(
+              ::features::kWorkerOrWorkletAgentDoubleReleaseFix) &&

diff --git a/content/browser/devtools/devtools_refcount_unittest.cc b/content/browser/devtools/devtools_refcount_unittest.cc
new file mode 100644
index 0000000..05da47f
--- /dev/null
+++ b/content/browser/devtools/devtools_refcount_unittest.cc
@@ -0,0 +1,93 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/test/bind.h"
+#include "content/browser/devtools/worker_or_worklet_devtools_agent_host.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/mock_render_process_host.h"
+#include "content/public/test/test_browser_context.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+class TestAgentHost : public WorkerOrWorkletDevToolsAgentHost {
+ public:
+  TestAgentHost(
+      int process_id,
+      const GURL& url,
+      const std::string& name,
+      const base::UnguessableToken& devtools_worker_token,
+      const std::string& parent_id,
+      base::OnceCallback<void(DevToolsAgentHostImpl*)> destroyed_callback)
+      : WorkerOrWorkletDevToolsAgentHost(process_id,
+                                         url,
+                                         name,
+                                         devtools_worker_token,
+                                         parent_id,
+                                         std::move(destroyed_callback)) {
+    NotifyCreated();
+  }
+
+  using WorkerOrWorkletDevToolsAgentHost::Disconnected;
+
+  // DevToolsAgentHost implementation:
+  std::string GetType() override { return "test"; }
+  bool AttachSession(DevToolsSession* session) override { return true; }
+  void DetachSession(DevToolsSession* session) override {}
+
+ private:
+  ~TestAgentHost() override = default;
+};
+
+}  // namespace
+
+class DevToolsRefcountTest : public testing::Test {
+ public:
+  DevToolsRefcountTest()
+      : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP) {}
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestBrowserContext browser_context_;
+};
+
+// Tests that WorkerOrWorkletDevToolsAgentHost doesn't underflow refcount
+// when Disconnected() is called multiple times.
+TEST_F(DevToolsRefcountTest, PreventDoubleRelease) {
+  MockRenderProcessHost renderer_host(&browser_context_);
+  const int process_id = renderer_host.GetDeprecatedID();
+  base::UnguessableToken token = base::UnguessableToken::Create();
+
+  bool destroyed_called = false;
+  auto destroyed_callback = base::BindLambdaForTesting(
+      [&](DevToolsAgentHostImpl* host) { destroyed_called = true; });
+
+  // 1. Create a TestAgentHost.
+  // It starts with refcount 1 (from our scoped_refptr) + 1 (from
+  // self-keep-alive in constructor).
+  scoped_refptr<TestAgentHost> agent_host = base::MakeRefCounted<TestAgentHost>(
+      process_id, GURL("http://example.com"), "worker", token, "parent_id",
+      std::move(destroyed_callback));
+
+  // 2. Call Disconnected() first time.
+  // This should trigger the first Release() and destroyed_callback.
+  agent_host->Disconnected();
+  EXPECT_TRUE(destroyed_called);
+
+  // 3. Call Disconnected() multiple times.
+  // If the fix is working, these subsequent calls won't cause double-release.
+  // We reset destroyed_called to ensure it's not called again.
+  destroyed_called = false;
+  agent_host->Disconnected();
+  agent_host->Disconnected();
+  EXPECT_FALSE(destroyed_called);
+
+  // 4. Verification: If we can still access agent_host, it hasn't been
+  // over-released. The refcount should be 1 (only our scoped_refptr).
+  EXPECT_TRUE(agent_host->HasOneRef());
+}
+
+}  // namespace content
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 12eea20b..9c8b29e 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -3677,6 +3677,7 @@
     # Non-Android.
     sources += [
       "../browser/devtools/devtools_manager_unittest.cc",
+      "../browser/devtools/devtools_refcount_unittest.cc",
       "../browser/devtools/protocol/webauthn_handler_unittest.cc",
       "../browser/host_zoom_map_impl_unittest.cc",
       "../browser/picture_in_picture/document_picture_in_picture_navigation_throttle_unittest.cc",