[Backport] CVE-2019-13665

Fix multiple download protection for <a download> x-origin redirect

The bug: multiple downloads protection is bypassed when there are multiple
<a download> download attempts and they end up triggering a x-origin redirect
to another download.

The cause: Each x-origin redirect following the <a download> is being treated as
a navigation. (See DownloadManagerImpl::InterceptDownload() (NetworkService
enabled), DownloadResourceHandler::OnRequestRedirected() (NetworkService
disabled)). The navigation will hit
DownloadRequestLimiter::TabDownloadState::DidStartNavigation that resets some
state of the limiter, and future downloads won't be prevented.

The solution: plumb |from_download_cross_origin_redirect| to NavigationRequest,
and skip resetting the limiter state when the flag is true.

Bug: 959640
Change-Id: I7d8aca09670be5258e149e34e3e6f2f3107442ff
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665973}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

9 files changed, 37 insertions, 1 deletions

diff --git a/chromium/content/browser/download/download_manager_impl.cc b/chromium/content/browser/download/download_manager_impl.cc
index 771bcf5ac57..7cb5bc49ca1 100644
--- a/chromium/content/browser/download/download_manager_impl.cc
+++ b/chromium/content/browser/download/download_manager_impl.cc
@@ -532,6 +532,7 @@ bool DownloadManagerImpl::InterceptDownload(
       params.frame_tree_node_id =
           RenderFrameHost::GetFrameTreeNodeIdForRoutingId(
               info.render_process_id, info.render_frame_id);
+      params.from_download_cross_origin_redirect = true;
       params.is_renderer_initiated = info.is_content_initiated;
       web_contents->GetController().LoadURLWithParams(params);
     }
diff --git a/chromium/content/browser/download/download_resource_handler.cc b/chromium/content/browser/download/download_resource_handler.cc
index 4548603991f..7863fc30a69 100644
--- a/chromium/content/browser/download/download_resource_handler.cc
+++ b/chromium/content/browser/download/download_resource_handler.cc
@@ -124,6 +124,7 @@ void NavigateOnUIThread(const GURL& url,
                         const std::vector<GURL> url_chain,
                         const Referrer& referrer,
                         bool has_user_gesture,
+                        bool from_download_cross_origin_redirect,
                         const ResourceRequestInfo::WebContentsGetter& wc_getter,
                         int frame_tree_node_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -135,6 +136,8 @@ void NavigateOnUIThread(const GURL& url,
     params.referrer = referrer;
     params.redirect_chain = url_chain;
     params.frame_tree_node_id = frame_tree_node_id;
+    params.from_download_cross_origin_redirect =
+        from_download_cross_origin_redirect;
     web_contents->GetController().LoadURLWithParams(params);
   }
 }
@@ -210,6 +213,7 @@ void DownloadResourceHandler::OnRequestRedirected(
                      Referrer::NetReferrerPolicyToBlinkReferrerPolicy(
                          redirect_info.new_referrer_policy)),
             GetRequestInfo()->HasUserGesture(),
+            true /* from_download_cross_origin_redirect */,
             GetRequestInfo()->GetWebContentsGetterForRequest(),
             GetRequestInfo()->frame_tree_node_id()));
     controller->Cancel();
diff --git a/chromium/content/browser/frame_host/navigation_controller_impl.cc b/chromium/content/browser/frame_host/navigation_controller_impl.cc
index e9e976db4f5..e1177fe329b 100644
--- a/chromium/content/browser/frame_host/navigation_controller_impl.cc
+++ b/chromium/content/browser/frame_host/navigation_controller_impl.cc
@@ -2990,10 +2990,14 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
   // extra_headers in params are \n separated; NavigationRequests want \r\n.
   std::string extra_headers_crlf;
   base::ReplaceChars(params.extra_headers, "\n", "\r\n", &extra_headers_crlf);
-  return NavigationRequest::CreateBrowserInitiated(
+
+  auto navigation_request = NavigationRequest::CreateBrowserInitiated(
       node, common_params, commit_params, !params.is_renderer_initiated,
       extra_headers_crlf, *frame_entry, entry, request_body,
       params.navigation_ui_data ? params.navigation_ui_data->Clone() : nullptr);
+  navigation_request->set_from_download_cross_origin_redirect(
+      params.from_download_cross_origin_redirect);
+  return navigation_request;
 }
 
 std::unique_ptr<NavigationRequest>
diff --git a/chromium/content/browser/frame_host/navigation_handle_impl.cc b/chromium/content/browser/frame_host/navigation_handle_impl.cc
index f4cf4f69bbe..15ce742c01f 100644
--- a/chromium/content/browser/frame_host/navigation_handle_impl.cc
+++ b/chromium/content/browser/frame_host/navigation_handle_impl.cc
@@ -568,6 +568,10 @@ const base::Optional<url::Origin>& NavigationHandleImpl::GetInitiatorOrigin() {
   return initiator_origin_;
 }
 
+bool NavigationHandleImpl::FromDownloadCrossOriginRedirect() {
+  return navigation_request_->from_download_cross_origin_redirect();
+}
+
 bool NavigationHandleImpl::IsSignedExchangeInnerResponse() {
   return is_signed_exchange_inner_response_;
 }
diff --git a/chromium/content/browser/frame_host/navigation_handle_impl.h b/chromium/content/browser/frame_host/navigation_handle_impl.h
index a882639df86..f0f811777de 100644
--- a/chromium/content/browser/frame_host/navigation_handle_impl.h
+++ b/chromium/content/browser/frame_host/navigation_handle_impl.h
@@ -133,6 +133,7 @@ class CONTENT_EXPORT NavigationHandleImpl : public NavigationHandle,
   const net::ProxyServer& GetProxyServer() override;
   const std::string& GetHrefTranslate() override;
   const base::Optional<url::Origin>& GetInitiatorOrigin() override;
+  bool FromDownloadCrossOriginRedirect() override;
 
   const std::string& origin_policy() const { return origin_policy_; }
   void set_origin_policy(const std::string& origin_policy) {
diff --git a/chromium/content/browser/frame_host/navigation_request.h b/chromium/content/browser/frame_host/navigation_request.h
index 0af28a81c9c..f64054baf90 100644
--- a/chromium/content/browser/frame_host/navigation_request.h
+++ b/chromium/content/browser/frame_host/navigation_request.h
@@ -197,6 +197,15 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate {
     on_start_checks_complete_closure_ = closure;
   }
 
+  bool from_download_cross_origin_redirect() const {
+    return from_download_cross_origin_redirect_;
+  }
+
+  void set_from_download_cross_origin_redirect(
+      bool from_download_cross_origin_redirect) {
+    from_download_cross_origin_redirect_ = from_download_cross_origin_redirect;
+  }
+
   int nav_entry_id() const { return nav_entry_id_; }
 
   // For automation driver-initiated navigations over the devtools protocol,
@@ -471,6 +480,10 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate {
   // Only valid when PerNavigationMojoInterface is enabled.
   mojom::NavigationClientAssociatedPtr commit_navigation_client_;
 
+  // Whether this navigation was triggered by a x-origin redirect following a
+  // prior (most likely <a download>) download attempt.
+  bool from_download_cross_origin_redirect_ = false;
+
   // If set, any redirects to HTTP for this navigation will be upgraded to
   // HTTPS. This is used only on subframe navigations, when
   // upgrade-insecure-requests is set as a CSP policy.
diff --git a/chromium/content/public/browser/navigation_controller.cc b/chromium/content/public/browser/navigation_controller.cc
index 28abe1171a9..7b11ca8f266 100644
--- a/chromium/content/public/browser/navigation_controller.cc
+++ b/chromium/content/public/browser/navigation_controller.cc
@@ -25,6 +25,7 @@ NavigationController::LoadURLParams::LoadURLParams(const GURL& url)
       should_clear_history_list(false),
       started_from_context_menu(false),
       navigation_ui_data(nullptr),
+      from_download_cross_origin_redirect(false),
       was_activated(WasActivatedOption::kUnknown),
       reload_type(ReloadType::NONE) {}
 
diff --git a/chromium/content/public/browser/navigation_controller.h b/chromium/content/public/browser/navigation_controller.h
index 949c91dd136..f8929f2945d 100644
--- a/chromium/content/public/browser/navigation_controller.h
+++ b/chromium/content/public/browser/navigation_controller.h
@@ -201,6 +201,10 @@ class NavigationController {
     // ContentBrowserClient::GetNavigationUIData.
     std::unique_ptr<NavigationUIData> navigation_ui_data;
 
+    // Whether this navigation was triggered by a x-origin redirect following a
+    // prior (most likely <a download>) download attempt.
+    bool from_download_cross_origin_redirect;
+
     // Time at which the input leading to this navigation occurred. This field
     // is set for links clicked by the user; the embedder is recommended to set
     // it for navigations it initiates.
diff --git a/chromium/content/public/browser/navigation_handle.h b/chromium/content/public/browser/navigation_handle.h
index d20c8207f4a..819892174f7 100644
--- a/chromium/content/public/browser/navigation_handle.h
+++ b/chromium/content/public/browser/navigation_handle.h
@@ -317,6 +317,10 @@ class CONTENT_EXPORT NavigationHandle {
   // Returns whether this navigation is currently deferred.
   virtual bool IsDeferredForTesting() = 0;
 
+  // Whether this navigation was triggered by a x-origin redirect following a
+  // prior (most likely <a download>) download attempt.
+  virtual bool FromDownloadCrossOriginRedirect() = 0;
+
   // The NavigationData that the embedder returned from
   // ResourceDispatcherHostDelegate::GetNavigationData during commit. This will
   // be a clone of the NavigationData.