From ada63371bafd041317141b7bf15674b5366df3fc Mon Sep 17 00:00:00 2001 From: Andrey Kosyakov Date: Fri, 10 Jan 2020 04:45:57 +0000 Subject: [Backport] CVE-2020-6392 - Insufficient policy enforcement in extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual backport of patch originally reviewed on: https://chromium-review.googlesource.com/c/chromium/src/+/1956529 https://chromium-review.googlesource.com/c/chromium/src/+/1992764: DevTools: check session can inspect URL we're about to navigate to TBR=rdevlin.cronin@chromium.org (cherry picked from commit 0788b1d419f78050f1114fffefd1f68cd88d1dab) Bug: 1030411 Change-Id: I31477509283e166cf51f58d617df95628dcb0f60 Reviewed-by: Jüri Valdmann --- .../extensions/api/debugger/debugger_api.cc | 25 +++++------------ .../browser/devtools/devtools_instrumentation.cc | 7 +++++ .../devtools/render_frame_devtools_agent_host.cc | 31 ++++++++++++++++------ .../devtools/render_frame_devtools_agent_host.h | 4 +++ .../public/browser/devtools_agent_host_client.cc | 4 +-- .../public/browser/devtools_agent_host_client.h | 7 +++-- 6 files changed, 44 insertions(+), 34 deletions(-) diff --git a/chromium/chrome/browser/extensions/api/debugger/debugger_api.cc b/chromium/chrome/browser/extensions/api/debugger/debugger_api.cc index e5cd5ab180e..56e94b5d685 100644 --- a/chromium/chrome/browser/extensions/api/debugger/debugger_api.cc +++ b/chromium/chrome/browser/extensions/api/debugger/debugger_api.cc @@ -146,8 +146,7 @@ class ExtensionDevToolsClientHost : public content::DevToolsAgentHostClient, void AgentHostClosed(DevToolsAgentHost* agent_host) override; void DispatchProtocolMessage(DevToolsAgentHost* agent_host, const std::string& message) override; - bool MayAttachToRenderer(content::RenderFrameHost* render_frame_host, - bool is_webui) override; + bool MayAttachToURL(const GURL& url, bool is_webui) override; bool MayAttachToBrowser() override; bool MayReadLocalFiles() override; bool MayWriteLocalFiles() override; @@ -358,29 +357,17 @@ void ExtensionDevToolsClientHost::DispatchProtocolMessage( } } -bool ExtensionDevToolsClientHost::MayAttachToRenderer( - content::RenderFrameHost* render_frame_host, - bool is_webui) { +bool ExtensionDevToolsClientHost::MayAttachToURL(const GURL& url, + bool is_webui) { if (is_webui) return false; - if (!render_frame_host) + // Allow the extension to attach to about:blank. + if (url.is_empty() || url == "about:") return true; std::string error; - // We check the site instance URL here (instead of - // RenderFrameHost::GetLastCommittedURL()) because it's too early in the - // navigation for anything else. - const GURL& site_instance_url = - render_frame_host->GetSiteInstance()->GetSiteURL(); - - if (site_instance_url.is_empty() || site_instance_url == "about:") { - // Allow the extension to attach to about:blank. - return true; - } - - return ExtensionCanAttachToURL(*extension_, site_instance_url, profile_, - &error); + return ExtensionCanAttachToURL(*extension_, url, profile_, &error); } bool ExtensionDevToolsClientHost::MayAttachToBrowser() { diff --git a/chromium/content/browser/devtools/devtools_instrumentation.cc b/chromium/content/browser/devtools/devtools_instrumentation.cc index 7fce34ad104..7ccedb25d06 100644 --- a/chromium/content/browser/devtools/devtools_instrumentation.cc +++ b/chromium/content/browser/devtools/devtools_instrumentation.cc @@ -369,6 +369,13 @@ bool WillCreateURLLoaderFactory( void OnNavigationRequestWillBeSent( const NavigationRequest& navigation_request) { + auto* agent_host = static_cast( + RenderFrameDevToolsAgentHost::GetFor( + navigation_request.frame_tree_node())); + if (!agent_host) + return; + agent_host->OnNavigationRequestWillBeSent(navigation_request); + DispatchToAgents(navigation_request.frame_tree_node(), &protocol::NetworkHandler::NavigationRequestWillBeSent, navigation_request); diff --git a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc index 85f51815313..ec0bfc836a6 100644 --- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc +++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -601,6 +601,19 @@ void RenderFrameDevToolsAgentHost::OnPageScaleFactorChanged( input->OnPageScaleFactorChanged(page_scale_factor); } +void RenderFrameDevToolsAgentHost::OnNavigationRequestWillBeSent( + const NavigationRequest& navigation_request) { + const auto& url = navigation_request.common_params().url; + std::vector restricted_sessions; + bool is_webui = frame_host_ && frame_host_->web_ui(); + for (DevToolsSession* session : sessions()) { + if (!session->client()->MayAttachToURL(url, is_webui)) + restricted_sessions.push_back(session); + } + if (!restricted_sessions.empty()) + ForceDetachRestrictedSessions(restricted_sessions); +} + void RenderFrameDevToolsAgentHost::DisconnectWebContents() { navigation_handles_.clear(); SetFrameTreeNode(nullptr); @@ -786,16 +799,18 @@ bool RenderFrameDevToolsAgentHost::IsChildFrame() { bool RenderFrameDevToolsAgentHost::ShouldAllowSession( DevToolsSession* session) { + // There's not much we can say if there's not host yet, but we'll + // check again when host is updated. + if (!frame_host_) + return true; DevToolsManager* manager = DevToolsManager::GetInstance(); - if (manager->delegate() && frame_host_) { - if (!manager->delegate()->AllowInspectingRenderFrameHost(frame_host_)) - return false; - } - const bool is_webui = - frame_host_ && (frame_host_->web_ui() || frame_host_->pending_web_ui()); - if (!session->client()->MayAttachToRenderer(frame_host_, is_webui)) + if (manager->delegate() && + !manager->delegate()->AllowInspectingRenderFrameHost(frame_host_)) { return false; - return true; + } + // Note this may be called before navigation is committed. + return session->client()->MayAttachToURL( + frame_host_->GetSiteInstance()->GetSiteURL(), frame_host_->web_ui()); } void RenderFrameDevToolsAgentHost::UpdateResourceLoaderFactories() { diff --git a/chromium/content/browser/devtools/render_frame_devtools_agent_host.h b/chromium/content/browser/devtools/render_frame_devtools_agent_host.h index 574e98f96be..d1120376609 100644 --- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.h +++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.h @@ -32,6 +32,7 @@ namespace content { class BrowserContext; class DevToolsFrameTraceRecorder; class FrameTreeNode; +class NavigationRequest; class NavigationHandleImpl; class RenderFrameHostImpl; struct DevToolsFrameMetadata; @@ -70,6 +71,9 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost const DevToolsFrameMetadata& frame_metadata); FrameTreeNode* frame_tree_node() { return frame_tree_node_; } + void OnNavigationRequestWillBeSent( + const NavigationRequest& navigation_request); + // DevToolsAgentHost overrides. void DisconnectWebContents() override; void ConnectWebContents(WebContents* web_contents) override; diff --git a/chromium/content/public/browser/devtools_agent_host_client.cc b/chromium/content/public/browser/devtools_agent_host_client.cc index 213b1d64b0b..3ac56e10213 100644 --- a/chromium/content/public/browser/devtools_agent_host_client.cc +++ b/chromium/content/public/browser/devtools_agent_host_client.cc @@ -6,9 +6,7 @@ namespace content { -bool DevToolsAgentHostClient::MayAttachToRenderer( - content::RenderFrameHost* render_frame_host, - bool is_webui) { +bool DevToolsAgentHostClient::MayAttachToURL(const GURL& url, bool is_webui) { return true; } diff --git a/chromium/content/public/browser/devtools_agent_host_client.h b/chromium/content/public/browser/devtools_agent_host_client.h index 3cd124c38b2..c447a2652e3 100644 --- a/chromium/content/public/browser/devtools_agent_host_client.h +++ b/chromium/content/public/browser/devtools_agent_host_client.h @@ -8,11 +8,11 @@ #include #include "content/common/content_export.h" +class GURL; namespace content { class DevToolsAgentHost; -class RenderFrameHost; // DevToolsAgentHostClient can attach to a DevToolsAgentHost and start // debugging it. @@ -27,10 +27,9 @@ class CONTENT_EXPORT DevToolsAgentHostClient { // This method is called when attached agent host is closed. virtual void AgentHostClosed(DevToolsAgentHost* agent_host) = 0; - // Returns true if the client is allowed to attach to the given renderer. + // Returns true if the client is allowed to attach to the given URL. // Note: this method may be called before navigation commits. - virtual bool MayAttachToRenderer(content::RenderFrameHost* render_frame_host, - bool is_webui); + virtual bool MayAttachToURL(const GURL& url, bool is_webui); // Returns true if the client is allowed to attach to the browser agent host. // Browser client is allowed to discover other DevTools targets and generally -- cgit v1.2.1