diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-10-17 11:12:26 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-10-21 08:13:47 +0000 |
commit | 1d21cbce407f1b8c0c98a250a62e93c0e20932dc (patch) | |
tree | 2fa516714dab7250670afaf0769468ae18e4d9a7 | |
parent | 2708f4fe1f1a4aca14d888303bbdc21bcf629a19 (diff) | |
download | qtwebengine-chromium-1d21cbce407f1b8c0c98a250a62e93c0e20932dc.tar.gz |
[Backport] CVE-2019-13678/CVE-2019-13681
Pass request initiator to check whether a download can proceed
Currently download use webcontents::GeURL() to check content settings.
But the download can actually be triggered by javascript from another
origin. This CL fixes the issue by passing the request initiator to
check the content settings.
Here is what included in this CL:
1. removed originating_web_contents param from TabDownloadState ctor,
this param is never used.
2. Adding an origin param to DownloadRequestLimiter::CanDownload() call,
and it will be used to check the content settings.
3. In DownloadRequestLimiter::CanDownloadImpl(), always do content
setting check first. This fixes a bug that any site can always
trigger a download first even if its automatic download setting is
blocked
4. For restricted origins, record their download status. So that we can
differentiate origins that are blocked and origins that require prompt.
BUG=970378
Change-Id: I6f7efc8b5c6b27ff3eaec1bb436c5ffbb8c8b26d
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672091}
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
3 files changed, 9 insertions, 1 deletions
diff --git a/chromium/content/browser/download/download_manager_impl.cc b/chromium/content/browser/download/download_manager_impl.cc index 10c22f88cf5..4991e15ee68 100644 --- a/chromium/content/browser/download/download_manager_impl.cc +++ b/chromium/content/browser/download/download_manager_impl.cc @@ -533,6 +533,7 @@ bool DownloadManagerImpl::InterceptDownload( RenderFrameHost::GetFrameTreeNodeIdForRoutingId( info.render_process_id, info.render_frame_id); params.from_download_cross_origin_redirect = true; + params.initiator_origin = info.request_initiator; params.is_renderer_initiated = info.is_content_initiated; web_contents->GetController().LoadURLWithParams(params); } @@ -886,6 +887,8 @@ void DownloadManagerImpl::InterceptNavigation( const GURL& url = resource_request->url; const std::string& method = resource_request->method; + base::Optional<url::Origin> request_initiator = + resource_request->request_initiator; ResourceRequestInfo::WebContentsGetter web_contents_getter = base::BindRepeating(WebContents::FromFrameTreeNodeId, frame_tree_node_id); @@ -899,6 +902,7 @@ void DownloadManagerImpl::InterceptNavigation( std::move(url_loader_client_endpoints)); delegate_->CheckDownloadAllowed(std::move(web_contents_getter), url, method, + std::move(request_initiator), std::move(on_download_checks_done)); } @@ -1390,6 +1394,7 @@ void DownloadManagerImpl::BeginDownloadInternal( if (delegate_) { delegate_->CheckDownloadAllowed(std::move(web_contents_getter), url, method, + base::nullopt /*request_initiator*/, std::move(on_can_download_checks_done)); return; } diff --git a/chromium/content/public/browser/download_manager_delegate.cc b/chromium/content/public/browser/download_manager_delegate.cc index b5963aa8b77..7a82f639df9 100644 --- a/chromium/content/public/browser/download_manager_delegate.cc +++ b/chromium/content/public/browser/download_manager_delegate.cc @@ -62,6 +62,7 @@ void DownloadManagerDelegate::CheckDownloadAllowed( const ResourceRequestInfo::WebContentsGetter& web_contents_getter, const GURL& url, const std::string& request_method, + base::Optional<url::Origin> request_initiator, CheckDownloadAllowedCallback check_download_allowed_cb) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(std::move(check_download_allowed_cb), true)); diff --git a/chromium/content/public/browser/download_manager_delegate.h b/chromium/content/public/browser/download_manager_delegate.h index a413d861473..900b7c9320d 100644 --- a/chromium/content/public/browser/download_manager_delegate.h +++ b/chromium/content/public/browser/download_manager_delegate.h @@ -10,13 +10,14 @@ #include "base/callback.h" #include "base/files/file_path.h" #include "base/logging.h" +#include "base/optional.h" #include "base/time/time.h" #include "components/download/public/common/download_danger_type.h" #include "components/download/public/common/download_item.h" #include "content/common/content_export.h" #include "content/public/browser/resource_request_info.h" #include "content/public/browser/save_page_type.h" - +#include "url/origin.h" namespace content { @@ -188,6 +189,7 @@ class CONTENT_EXPORT DownloadManagerDelegate { const ResourceRequestInfo::WebContentsGetter& web_contents_getter, const GURL& url, const std::string& request_method, + base::Optional<url::Origin> request_initiator, CheckDownloadAllowedCallback check_download_allowed_cb); protected: |