diff options
author | Andrey Kosyakov <caseq@chromium.org> | 2020-08-21 19:31:34 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2020-09-22 11:56:06 +0000 |
commit | 295feb590b113a07ceec63a96420cca122a6aa01 (patch) | |
tree | a73f62e17438c3b2bf0239bb941140d3fdde2fd1 | |
parent | 405e7526583ae12d111fb57ae642157174ff9e65 (diff) | |
download | qtwebengine-chromium-295feb590b113a07ceec63a96420cca122a6aa01.tar.gz |
[Backport] CVE-2020-15963 and CVE-2020-15966
Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2368446:
Reland "Add more checks for chrome.debugger extensions"
This reverts commit 5a809a08fd5ca32cb8d594664416db2f2dc8ebdc.
Reason for revert: I don't think the test failure is related. Please
note it stopped before the revert landed (build no 91007 vs. 91010).
This must have been a flake, or a independent failure that has been
fixed by one of the front-end rolls.
Original change's description:
> Revert "Add more checks for chrome.debugger extensions"
>
> This reverts commit 4838b76ae48797760fd8a362b4dc15325ccddcf5.
>
> Reason for revert: 1119297
>
> Original change's description:
> > Add more checks for chrome.debugger extensions
> >
> > Bug: 1113558, 1113565
> > Change-Id: I99f2e030f9a38f1ffd6b6adc760ba15e5d231f96
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342277
> > Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
> > Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> > Reviewed-by: Yang Guo <yangguo@chromium.org>
> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> > Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#799514}
>
> TBR=dgozman@chromium.org,rdevlin.cronin@chromium.org,caseq@chromium.org,yangguo@chromium.org,sigurds@chromium.org
>
> Change-Id: I01ad12ca99ac75197f9073e2c6c9d0eaa0d95147
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1113558
> Bug: 1113565
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362920
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: Christian Dullweber <dullweber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#799558}
TBR=dgozman@chromium.org,rdevlin.cronin@chromium.org,caseq@chromium.org,yangguo@chromium.org,sigurds@chromium.org,dullweber@chromium.org
Bug: 1113558
Bug: 1113565
Change-Id: Ic98fc037028a210204b7935b0b8e50e4e36e2397
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800682}
Change-Id: Ie63da838a9432e8e49218ba774204fd22a3ce5ce
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
3 files changed, 42 insertions, 14 deletions
diff --git a/chromium/content/browser/devtools/devtools_instrumentation.cc b/chromium/content/browser/devtools/devtools_instrumentation.cc index 91da6cb62a5..04db4e1e68d 100644 --- a/chromium/content/browser/devtools/devtools_instrumentation.cc +++ b/chromium/content/browser/devtools/devtools_instrumentation.cc @@ -440,12 +440,24 @@ bool WillCreateURLLoaderFactory( void OnNavigationRequestWillBeSent( const NavigationRequest& navigation_request) { - auto* agent_host = static_cast<RenderFrameDevToolsAgentHost*>( - RenderFrameDevToolsAgentHost::GetFor( - navigation_request.frame_tree_node())); - if (!agent_host) - return; - agent_host->OnNavigationRequestWillBeSent(navigation_request); + // Note this intentionally deviates from the usual instrumentation signal + // logic and dispatches to all agents upwards from the frame, to make sure + // the security checks are properly applied even if no DevTools session is + // established for the navigated frame itself. This is because the page + // agent may navigate all of its subframes currently. + for (RenderFrameHostImpl* rfh = + navigation_request.frame_tree_node()->current_frame_host(); + rfh; rfh = rfh->GetParent()) { + // Only check frames that qualify as DevTools targets, i.e. (local)? roots. + if (!RenderFrameDevToolsAgentHost::ShouldCreateDevToolsForHost(rfh)) + continue; + auto* agent_host = static_cast<RenderFrameDevToolsAgentHost*>( + RenderFrameDevToolsAgentHost::GetFor(rfh)); + if (!agent_host) + continue; + agent_host->OnNavigationRequestWillBeSent(navigation_request); + } + // Make sure both back-ends yield the same timestamp. auto timestamp = base::TimeTicks::Now(); 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 41cfb1af2c9..cb8a18c5a40 100644 --- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc +++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -86,10 +86,6 @@ RenderFrameDevToolsAgentHost* FindAgentHost(FrameTreeNode* frame_tree_node) { return it == g_agent_host_instances.Get().end() ? nullptr : it->second; } -bool ShouldCreateDevToolsForHost(RenderFrameHost* rfh) { - return rfh->IsCrossProcessSubframe() || !rfh->GetParent(); -} - bool ShouldCreateDevToolsForNode(FrameTreeNode* ftn) { return !ftn->parent() || ftn->current_frame_host()->IsCrossProcessSubframe(); } @@ -141,6 +137,12 @@ scoped_refptr<DevToolsAgentHost> RenderFrameDevToolsAgentHost::GetOrCreateFor( } // static +bool RenderFrameDevToolsAgentHost::ShouldCreateDevToolsForHost( + RenderFrameHost* rfh) { + return rfh->IsCrossProcessSubframe() || !rfh->GetParent(); +} + +// static scoped_refptr<DevToolsAgentHost> RenderFrameDevToolsAgentHost::CreateForCrossProcessNavigation( NavigationRequest* request) { @@ -627,7 +629,9 @@ void RenderFrameDevToolsAgentHost::OnPageScaleFactorChanged( void RenderFrameDevToolsAgentHost::OnNavigationRequestWillBeSent( const NavigationRequest& navigation_request) { - const auto& url = navigation_request.common_params().url; + GURL url = navigation_request.common_params().url; + if (url.SchemeIs(url::kJavaScriptScheme) && frame_host_) + url = frame_host_->GetLastCommittedURL(); std::vector<DevToolsSession*> restricted_sessions; bool is_webui = frame_host_ && frame_host_->web_ui(); for (DevToolsSession* session : sessions()) { @@ -837,9 +841,16 @@ bool RenderFrameDevToolsAgentHost::ShouldAllowSession( !manager->delegate()->AllowInspectingRenderFrameHost(frame_host_)) { return false; } - // Note this may be called before navigation is committed. - return session->GetClient()->MayAttachToURL( - frame_host_->GetSiteInstance()->GetSiteURL(), frame_host_->web_ui()); + auto* root = static_cast<RenderFrameHostImpl*>(frame_host_)->frame_tree_node(); + 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())) { + return false; + } + } + return true; } 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 69a66009454..30ae26634fb 100644 --- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.h +++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.h @@ -61,6 +61,11 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost static scoped_refptr<DevToolsAgentHost> GetOrCreateFor( FrameTreeNode* frame_tree_node); + // Whether the RFH passed may have associated DevTools agent host + // (i.e. the specified RFH is a local root). This does not indicate + // whether DevToolsAgentHost has actually been created. + static bool ShouldCreateDevToolsForHost(RenderFrameHost* rfh); + // This method is called when new frame is created during cross process // navigation. static scoped_refptr<DevToolsAgentHost> CreateForCrossProcessNavigation( |