[Backport] CVE-2019-13754: Insufficient policy enforcement in extensions (2/3)

Manual backport of patch:
Remove ContentBrowserClient::IsSafeRedirectTarget.

The deleted ChromeContentBrowserClient::IsSafeRedirectTarget was mostly
redundant wrt ExtensionNavigationThrottle ("mostly", because the
throttle considers only the navigation initiator - doesn't consider
origins of intermediate hops as initiators;  this aspect of behavior is
not important to preserve).

Bug: 442579
Change-Id: I7a8e36b6152788ca63507f09ba368ae75074e0cf
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

4 files changed, 6 insertions, 41 deletions

diff --git a/chromium/content/browser/loader/navigation_url_loader_impl.cc b/chromium/content/browser/loader/navigation_url_loader_impl.cc
index 3d6c804d28e..1341eba3b01 100644
--- a/chromium/content/browser/loader/navigation_url_loader_impl.cc
+++ b/chromium/content/browser/loader/navigation_url_loader_impl.cc
@@ -4,7 +4,9 @@
 
 #include "content/browser/loader/navigation_url_loader_impl.h"
 
+#include <map>
 #include <memory>
+#include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ -307,21 +309,6 @@ void UnknownSchemeCallback(
       handled_externally ? net::ERR_ABORTED : net::ERR_UNKNOWN_URL_SCHEME));
 }
 
-// Determines whether it is safe to redirect from |from_url| to |to_url|.
-bool IsRedirectSafe(const GURL& from_url,
-                    const GURL& to_url,
-                    BrowserContext* browser_context,
-                    ResourceContext* resource_context) {
-  if (NavigationURLLoaderImpl::IsNavigationLoaderOnUIEnabled()) {
-    return IsSafeRedirectTarget(from_url, to_url) &&
-           GetContentClient()->browser()->IsSafeRedirectTarget(to_url,
-                                                               browser_context);
-  }
-  return IsSafeRedirectTarget(from_url, to_url) &&
-         GetContentClient()->browser()->IsSafeRedirectTargetOnIO(
-             to_url, resource_context);
-}
-
 // Runs |task| on the thread specified by |thread_id| if already on that thread,
 // otherwise posts a task to that thread.
 void RunOrPostTaskIfNecessary(const base::Location& from_here,
@@ -1366,8 +1353,7 @@ class NavigationURLLoaderImpl::URLLoaderRequestController
                          const network::ResourceResponseHead& head) override {
     if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
         !bypass_redirect_checks_ &&
-        !IsRedirectSafe(url_, redirect_info.new_url, browser_context_,
-                        resource_context_)) {
+        !IsSafeRedirectTarget(url_, redirect_info.new_url)) {
       // Call CancelWithError instead of OnComplete so that if there is an
       // intercepting URLLoaderFactory (created through the embedder's
       // ContentBrowserClient::WillCreateURLLoaderFactory) it gets notified.
diff --git a/chromium/content/public/browser/content_browser_client.cc b/chromium/content/public/browser/content_browser_client.cc
index 78770a86401..742d3b7a50b 100644
--- a/chromium/content/public/browser/content_browser_client.cc
+++ b/chromium/content/public/browser/content_browser_client.cc
@@ -908,16 +908,6 @@ ContentBrowserClient::CreateWindowForPictureInPicture(
   return nullptr;
 }
 
-bool ContentBrowserClient::IsSafeRedirectTargetOnIO(const GURL& url,
-                                                    ResourceContext* context) {
-  return true;
-}
-
-bool ContentBrowserClient::IsSafeRedirectTarget(const GURL& url,
-                                                BrowserContext* context) {
-  return true;
-}
-
 void ContentBrowserClient::RegisterRendererPreferenceWatcher(
     BrowserContext* browser_context,
     blink::mojom::RendererPreferenceWatcherPtr watcher) {
diff --git a/chromium/content/public/browser/content_browser_client.h b/chromium/content/public/browser/content_browser_client.h
index ec27c9eca33..0d1f322960b 100644
--- a/chromium/content/public/browser/content_browser_client.h
+++ b/chromium/content/public/browser/content_browser_client.h
@@ -1511,13 +1511,6 @@ class CONTENT_EXPORT ContentBrowserClient {
   virtual std::unique_ptr<OverlayWindow> CreateWindowForPictureInPicture(
       PictureInPictureWindowController* controller);
 
-  // Returns true if it is safe to redirect to |url|, otherwise returns false.
-  // This is called on the IO thread.
-  virtual bool IsSafeRedirectTargetOnIO(const GURL& url,
-                                        ResourceContext* context);
-  // Same as above but called on UI thread.
-  virtual bool IsSafeRedirectTarget(const GURL& url, BrowserContext* context);
-
   // Registers the watcher to observe updates in RendererPreferences.
   virtual void RegisterRendererPreferenceWatcher(
       BrowserContext* browser_context,
diff --git a/chromium/extensions/browser/extension_navigation_throttle.cc b/chromium/extensions/browser/extension_navigation_throttle.cc
index ef8c828700e..fc7c2d221cd 100644
--- a/chromium/extensions/browser/extension_navigation_throttle.cc
+++ b/chromium/extensions/browser/extension_navigation_throttle.cc
@@ -4,6 +4,8 @@
 
 #include "extensions/browser/extension_navigation_throttle.h"
 
+#include <string>
+
 #include "components/guest_view/browser/guest_view_base.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
@@ -189,13 +191,7 @@ ExtensionNavigationThrottle::WillStartRequest() {
 
 content::NavigationThrottle::ThrottleCheckResult
 ExtensionNavigationThrottle::WillRedirectRequest() {
-  ThrottleCheckResult result = WillStartOrRedirectRequest();
-  if (result.action() == BLOCK_REQUEST) {
-    // TODO(nick): https://crbug.com/695421 means that BLOCK_REQUEST does not
-    // work here. Once PlzNavigate is enabled 100%, just return |result|.
-    return CANCEL;
-  }
-  return result;
+  return WillStartOrRedirectRequest();
 }
 
 const char* ExtensionNavigationThrottle::GetNameForLogging() {