From de3302be59dad44f4dc1e84bceb30e3c8dc74b5c Mon Sep 17 00:00:00 2001
From: Danil Somsikov <dsv@chromium.org>
Date: Mon, 11 May 2026 05:21:00 -0700
Subject: [PATCH] Enforce security checks in DevTools child sessions and strengthen extension trust.

This change addresses a vulnerability where child sessions created via
the DevTools Target domain could bypass security restrictions normally
enforced on the root client.

Specifically:

1. Enforce checks in child sessions: TargetHandler::Session now
overrides MayAttachToRenderFrameHost, MayAttachToURL, and
MayAccessAllCookies to delegate these checks to the root client. This
prevents a compromised renderer from using child sessions to bypass
WebUI attachment or cookie access restrictions.

2. Strengthen extension trust: ExtensionIsTrusted in debugger_api.cc is
updated to verify that the extension is not from an unpacked location.
This prevents attackers from gaining "trusted" status by loading an
unpacked extension with the Perfetto UI extension ID.

3. Improve robustness: TargetHandler::Session::Attach now returns a
std::optional<std::string> to provide more robust error handling when
session creation fails.

Bug: 505933538
Change-Id: I00dad3f1515032eca5296e9aa2876bed2bf5c851
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7802063
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Auto-Submit: Danil Somsikov <dsv@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1628501}
---

diff --git a/chrome/browser/extensions/api/debugger/debugger_api.cc b/chrome/browser/extensions/api/debugger/debugger_api.cc
index f69e9e3..666fd5f7 100644
--- a/chrome/browser/extensions/api/debugger/debugger_api.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_api.cc
@@ -33,6 +33,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_observer.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "components/security_interstitials/content/security_interstitial_tab_helper.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/navigation_entry.h"
@@ -50,6 +51,7 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_id.h"
+#include "extensions/common/manifest.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "pdf/buildflags.h"
@@ -225,10 +227,13 @@
 
 constexpr char kBrowserTargetId[] = "browser";
 
-constexpr char kPerfettoUIExtensionId[] = "lfmkphfpdbjijhpomgecfikhfohaoine";
-
 bool ExtensionIsTrusted(const Extension& extension) {
-  return extension.id() == kPerfettoUIExtensionId;
+  if (extension.id() != extension_misc::kPerfettoUIExtensionId) {
+    return false;
+  }
+  return !Manifest::IsUnpackedLocation(extension.location()) ||
+         base::CommandLine::ForCurrentProcess()->HasSwitch(
+             switches::kAllowUnpackedPerfettoExtension);
 }
 
 bool ExtensionMayAttachToRenderFrameHost(
diff --git a/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
index 4a1981c..4570311 100644
--- a/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
@@ -34,6 +34,7 @@
 #include "chrome/browser/ui/browser_window/public/browser_window_interface.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "chrome/test/base/browser_closed_waiter.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/infobars/content/content_infobar_manager.h"
@@ -325,6 +326,119 @@
 }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetAllowedForUnpackedPerfettoUIWithFlag) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      ::switches::kAllowUnpackedPerfettoExtension);
+
+  scoped_refptr<const Extension> unpacked_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kUnpacked)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(unpacked_perfetto.get());
+
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile()))
+      << attach_function->GetError();
+
+  // Clean up and detach.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(unpacked_perfetto.get());
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      detach_function.get(), R"([{"targetId": "browser"}])", profile()));
+}
+
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetNotAllowedForUnpackedPerfettoUI) {
+  scoped_refptr<const Extension> unpacked_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kUnpacked)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(unpacked_perfetto.get());
+
+  std::string actual_error = api_test_utils::RunFunctionAndReturnError(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile());
+
+  EXPECT_EQ("No target with given id browser.", actual_error);
+}
+
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetAllowedForComponentPerfettoUI) {
+  scoped_refptr<const Extension> component_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kComponent)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(component_perfetto.get());
+
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile()))
+      << attach_function->GetError();
+
+  // Now, try to attach to a WebUI page via Target.attachToTarget through the
+  // browser target (which acts as a root session). This should be blocked by
+  // the child session's delegated MayAttachToRenderFrameHost check.
+
+  // 1. Open a WebUI tab.
+  content::WebContents* web_contents = GetActiveWebContents();
+  ASSERT_TRUE(NavigateToURL(web_contents, GURL("chrome://version")));
+  int tab_id = sessions::SessionTabHelper::IdForTab(web_contents).id();
+
+  // 2. Find the targetId for the WebUI tab.
+  scoped_refptr<DebuggerGetTargetsFunction> get_targets =
+      new DebuggerGetTargetsFunction();
+  std::optional<base::Value> targets_value(
+      api_test_utils::RunFunctionAndReturnSingleResult(get_targets.get(), "[]",
+                                                       profile()));
+  ASSERT_TRUE(targets_value->is_list());
+
+  std::string webui_target_id;
+  for (const base::Value& target_value : targets_value->GetList()) {
+    std::optional<int> id = target_value.GetDict().FindInt("tabId");
+    if (id == tab_id) {
+      const std::string* id_str = target_value.GetDict().FindString("id");
+      ASSERT_TRUE(id_str);
+      webui_target_id = *id_str;
+      break;
+    }
+  }
+  ASSERT_FALSE(webui_target_id.empty());
+
+  // 3. Send Target.attachToTarget.
+  auto send_command = base::MakeRefCounted<DebuggerSendCommandFunction>();
+  send_command->set_extension(component_perfetto.get());
+
+  std::string command_args = base::StringPrintf(
+      R"([{"targetId": "browser"}, "Target.attachToTarget", )"
+      R"({"targetId": "%s"}])",
+      webui_target_id.c_str());
+
+  // Run the command and expect it to fail (it will return an error response).
+  std::string attach_error = api_test_utils::RunFunctionAndReturnError(
+      send_command.get(), command_args, profile());
+
+  // The attach should fail with an error because the delegated
+  // MayAttachToRenderFrameHost check will block attaching to the WebUI frame.
+  EXPECT_THAT(attach_error, testing::HasSubstr("Not allowed"));
+
+  // Clean up and detach.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(component_perfetto.get());
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      detach_function.get(), R"([{"targetId": "browser"}])", profile()));

diff --git a/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
index 4a1981c..4570311 100644
--- a/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
@@ -34,6 +34,7 @@
 #include "chrome/browser/ui/browser_window/public/browser_window_interface.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "chrome/test/base/browser_closed_waiter.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/infobars/content/content_infobar_manager.h"
@@ -325,6 +326,119 @@
 }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetAllowedForUnpackedPerfettoUIWithFlag) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      ::switches::kAllowUnpackedPerfettoExtension);
+
+  scoped_refptr<const Extension> unpacked_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kUnpacked)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(unpacked_perfetto.get());
+
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile()))
+      << attach_function->GetError();
+
+  // Clean up and detach.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(unpacked_perfetto.get());
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      detach_function.get(), R"([{"targetId": "browser"}])", profile()));
+}
+
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetNotAllowedForUnpackedPerfettoUI) {
+  scoped_refptr<const Extension> unpacked_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kUnpacked)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(unpacked_perfetto.get());
+
+  std::string actual_error = api_test_utils::RunFunctionAndReturnError(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile());
+
+  EXPECT_EQ("No target with given id browser.", actual_error);
+}
+
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       BrowserTargetAllowedForComponentPerfettoUI) {
+  scoped_refptr<const Extension> component_perfetto =
+      ExtensionBuilder("Perfetto UI")
+          .SetID(extension_misc::kPerfettoUIExtensionId)
+          .SetLocation(mojom::ManifestLocation::kComponent)
+          .AddAPIPermission("debugger")
+          .Build();
+
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(component_perfetto.get());
+
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      attach_function.get(), R"([{"targetId": "browser"}, "1.1"])", profile()))
+      << attach_function->GetError();
+
+  // Now, try to attach to a WebUI page via Target.attachToTarget through the
+  // browser target (which acts as a root session). This should be blocked by
+  // the child session's delegated MayAttachToRenderFrameHost check.
+
+  // 1. Open a WebUI tab.
+  content::WebContents* web_contents = GetActiveWebContents();
+  ASSERT_TRUE(NavigateToURL(web_contents, GURL("chrome://version")));
+  int tab_id = sessions::SessionTabHelper::IdForTab(web_contents).id();
+
+  // 2. Find the targetId for the WebUI tab.
+  scoped_refptr<DebuggerGetTargetsFunction> get_targets =
+      new DebuggerGetTargetsFunction();
+  std::optional<base::Value> targets_value(
+      api_test_utils::RunFunctionAndReturnSingleResult(get_targets.get(), "[]",
+                                                       profile()));
+  ASSERT_TRUE(targets_value->is_list());
+
+  std::string webui_target_id;
+  for (const base::Value& target_value : targets_value->GetList()) {
+    std::optional<int> id = target_value.GetDict().FindInt("tabId");
+    if (id == tab_id) {
+      const std::string* id_str = target_value.GetDict().FindString("id");
+      ASSERT_TRUE(id_str);
+      webui_target_id = *id_str;
+      break;
+    }
+  }
+  ASSERT_FALSE(webui_target_id.empty());
+
+  // 3. Send Target.attachToTarget.
+  auto send_command = base::MakeRefCounted<DebuggerSendCommandFunction>();
+  send_command->set_extension(component_perfetto.get());
+
+  std::string command_args = base::StringPrintf(
+      R"([{"targetId": "browser"}, "Target.attachToTarget", )"
+      R"({"targetId": "%s"}])",
+      webui_target_id.c_str());
+
+  // Run the command and expect it to fail (it will return an error response).
+  std::string attach_error = api_test_utils::RunFunctionAndReturnError(
+      send_command.get(), command_args, profile());
+
+  // The attach should fail with an error because the delegated
+  // MayAttachToRenderFrameHost check will block attaching to the WebUI frame.
+  EXPECT_THAT(attach_error, testing::HasSubstr("Not allowed"));
+
+  // Clean up and detach.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(component_perfetto.get());
+  EXPECT_TRUE(api_test_utils::RunFunction(
+      detach_function.get(), R"([{"targetId": "browser"}])", profile()));
+}
+
 class TestInterstitialPage
     : public security_interstitials::SecurityInterstitialPage {
  public: