summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAllan Sandfeld Jensen <allan.jensen@qt.io>2019-10-17 11:12:26 +0200
committerAllan Sandfeld Jensen <allan.jensen@qt.io>2019-10-21 08:13:47 +0000
commit1d21cbce407f1b8c0c98a250a62e93c0e20932dc (patch)
tree2fa516714dab7250670afaf0769468ae18e4d9a7
parent2708f4fe1f1a4aca14d888303bbdc21bcf629a19 (diff)
downloadqtwebengine-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>
-rw-r--r--chromium/content/browser/download/download_manager_impl.cc5
-rw-r--r--chromium/content/public/browser/download_manager_delegate.cc1
-rw-r--r--chromium/content/public/browser/download_manager_delegate.h4
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: