chrome.debugger: add more access checks

- check inner URL for nested URLs;
- check actual RFH of RenderFrameDevToolsAgentHost;

Bug: 1125362
Change-Id: I6d593d45d749b2a34d7711a983e2bda730027117
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506354Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823249}

chrome/browser/extensions/api/debugger/debugger_api.cc

@@ -405,7 +405,15 @@ bool ExtensionDevToolsClientHost::MayAttachToURL(const GURL& url,
   if (url.is_empty() || url == "about:")
     return true;
   std::string error;
-  return ExtensionMayAttachToURL(*extension_, url, profile_, &error);
+  if (!ExtensionMayAttachToURL(*extension_, url, profile_, &error))
+    return false;
+  // For nested URLs, make sure ExtensionMayAttachToURL() allows both
+  // the outer and the inner URLs.
+  if (url.inner_url() && !ExtensionMayAttachToURL(*extension_, *url.inner_url(),
+                                                  profile_, &error)) {
+    return false;
+  }
+  return true;
 }
 
 bool ExtensionDevToolsClientHost::MayAttachToBrowser() {

chrome/browser/extensions/api/debugger/debugger_apitest.cc

@@ -423,7 +423,7 @@ class DebuggerExtensionApiTest : public ExtensionApiTest {
     ExtensionApiTest::SetUpOnMainThread();
     host_resolver()->AddRule("*", "127.0.0.1");
     embedded_test_server()->ServeFilesFromSourceDirectory("chrome/test/data");
-    ASSERT_TRUE(embedded_test_server()->Start());
+    ASSERT_TRUE(StartEmbeddedTestServer());
   }
 };
 
@@ -499,4 +499,9 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessDebuggerExtensionApiTest,
       << message_;
 }
 
+IN_PROC_BROWSER_TEST_F(SitePerProcessDebuggerExtensionApiTest,
+                       DebuggerCheckInnerUrl) {
+  ASSERT_TRUE(RunExtensionTest("debugger_check_inner_url")) << message_;
+}
+
 }  // namespace extensions

chrome/test/data/extensions/api_test/debugger_check_inner_url/background.js

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const protocolVersion = '1.3';
+const DETACHED_WHILE_HANDLING = 'Detached while handling command.';
+
+async function findTarget(url) {
+  const targets = await new Promise(resolve =>
+      chrome.debugger.getTargets(resolve));
+  return targets.find(target => url === target.url);
+}
+
+chrome.test.getConfig(config => chrome.test.runTests([
+  async function testAccessToNavigationTarget() {
+    const {openTab} = await import('/_test_resources/test_util/tabs_util.js');
+    const pagePath = 'extensions/api_test/debugger_check_inner_url/page.html';
+    const topURL = `http://a.com:${config.testServer.port}/${pagePath}`;
+    const subframeURL = `http://b.com:${config.testServer.port}/${pagePath}`;
+    const tab = await openTab(topURL);
+    const debuggee = {tabId: tab.id};
+    await new Promise(resolve =>
+        chrome.debugger.attach(debuggee, protocolVersion, resolve));
+
+    const expression = `
+      new Promise(resolve => {
+        const frame = document.body.firstElementChild;
+        frame.onload = resolve;
+        frame.src = '${subframeURL}';
+      })
+    `;
+    await new Promise(resolve =>
+      chrome.debugger.sendCommand(debuggee, 'Runtime.evaluate', {
+          expression,
+          awaitPromise: true
+      }, resolve));
+
+    const subframeTarget = await findTarget(subframeURL);
+    const subframeDebuggee = {targetId: subframeTarget.id};
+    await new Promise(resolve =>
+        chrome.debugger.attach(subframeDebuggee, protocolVersion, resolve));
+
+    await new Promise(resolve =>
+        chrome.debugger.sendCommand(subframeDebuggee, 'Page.navigate', {
+            url: 'filesystem:chrome://version/non-existent/'}, resolve));
+
+    chrome.test.assertLastError(DETACHED_WHILE_HANDLING);
+
+    chrome.test.succeed();
+  }
+]));

chrome/test/data/extensions/api_test/debugger_check_inner_url/manifest.json

+{
+  "name": "Debugger API test for subframe navigation to composite URLs",
+  "version": "1.0",
+  "manifest_version": 2,
+  "background": {
+    "scripts": ["background.js"]
+  },
+  "permissions": [
+    "debugger"
+  ]
+}

chrome/test/data/extensions/api_test/debugger_check_inner_url/page.html

+<html>
+<iframe></iframe>
+</html>
\ No newline at end of file

content/browser/devtools/render_frame_devtools_agent_host.cc

@@ -842,14 +842,21 @@ bool RenderFrameDevToolsAgentHost::ShouldAllowSession(
       !manager->delegate()->AllowInspectingRenderFrameHost(frame_host_)) {
     return false;
   }
+  // In case this is called during the navigation, besides checking the
+  // access to the entire current local tree (below), ensure we can access
+  // the target URL.
+  const GURL& target_url = frame_host_->GetSiteInstance()->GetSiteURL();
+  if (!session->GetClient()->MayAttachToURL(target_url,
+                                            frame_host_->web_ui())) {
+    return false;
+  }
   auto* root = FrameTreeNode::From(frame_host_);
   for (FrameTreeNode* node : root->frame_tree()->SubtreeNodes(root)) {
     // Note this may be called before navigation is committed.
     RenderFrameHostImpl* rfh = node->current_frame_host();
     const GURL& url = rfh->GetSiteInstance()->GetSiteURL();
-    if (!session->GetClient()->MayAttachToURL(url, rfh->web_ui())) {
+    if (!session->GetClient()->MayAttachToURL(url, rfh->web_ui()))
       return false;
-    }
   }
   return true;
 }