[Backport] CVE-2019-13692

Require dedicated process for all WebUI schemes.

This changes SiteInstanceImpl::DoesSiteURLRequireDedicatedProcess() to
return true for all WebUI schemes instead of just singling out the
chrome: scheme. This ensures that these URLs get placed in dedicated
processes even if site isolation is disabled.

(cherry picked from commit 7be7426134cc4978a253f3be6dcdbf77ee25702f)

Bug: 991153,991888
Change-Id: I1af3b87ac39d93f6e45587a5b3845a176f98b7bd
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#689561}
Reviewed-by: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/branch-heads/3865@{#595}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 75 insertions, 4 deletions

diff --git a/chromium/content/browser/site_instance_impl.cc b/chromium/content/browser/site_instance_impl.cc
index e1a7560631b..0ec7f76e313 100644
--- a/chromium/content/browser/site_instance_impl.cc
+++ b/chromium/content/browser/site_instance_impl.cc
@@ -16,6 +16,7 @@
 #include "content/browser/isolation_context.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/storage_partition_impl.h"
+#include "content/browser/webui/url_data_manager_backend.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host_factory.h"
 #include "content/public/browser/site_isolation_policy.h"
@@ -608,10 +609,11 @@ bool SiteInstanceImpl::DoesSiteRequireDedicatedProcess(
   if (site_url.SchemeIs(kChromeErrorScheme))
     return true;
 
-  // Isolate kChromeUIScheme pages from one another and from other kinds of
-  // schemes.
-  if (site_url.SchemeIs(content::kChromeUIScheme))
-    return true;
+  // Isolate WebUI pages from one another and from other kinds of schemes.
+  for (const auto& webui_scheme : URLDataManagerBackend::GetWebUISchemes()) {
+    if (site_url.SchemeIs(webui_scheme))
+      return true;
+  }
 
   // Let the content embedder enable site isolation for specific URLs. Use the
   // canonical site url for this check, so that schemes with nested origins
diff --git a/chromium/content/browser/site_instance_impl_unittest.cc b/chromium/content/browser/site_instance_impl_unittest.cc
index 90a2d2ab5ed..bc83fd5f54b 100644
--- a/chromium/content/browser/site_instance_impl_unittest.cc
+++ b/chromium/content/browser/site_instance_impl_unittest.cc
@@ -1261,4 +1261,73 @@ TEST_F(SiteInstanceTest, IsOriginLockASite) {
       GURL("http://user:pass@google.com:99/foo;bar?q=a#ref")));
 }
 
+TEST_F(SiteInstanceTest, DoesSiteRequireDedicatedProcess) {
+  class CustomBrowserClient : public EffectiveURLContentBrowserClient {
+   public:
+    CustomBrowserClient(const GURL& url_to_modify,
+                        const GURL& url_to_return,
+                        bool requires_dedicated_process,
+                        const std::string& additional_webui_scheme)
+        : EffectiveURLContentBrowserClient(url_to_modify,
+                                           url_to_return,
+                                           requires_dedicated_process),
+          additional_webui_scheme_(additional_webui_scheme) {
+      DCHECK(!additional_webui_scheme.empty());
+    }
+
+   private:
+    void GetAdditionalWebUISchemes(
+        std::vector<std::string>* additional_schemes) override {
+      additional_schemes->push_back(additional_webui_scheme_);
+    }
+
+    const std::string additional_webui_scheme_;
+  };
+
+  const std::vector<std::string> kUrlsThatDoNotRequireADedicatedProcess = {
+      "about:blank",
+      "http://foo.com",
+      "data:text/html,Hello World!",
+      "file:///tmp/test.txt",
+  };
+
+  const char* kExplicitlyIsolatedURL = "http://isolated.com";
+  const char* kCustomWebUIScheme = "my-webui";
+  const char* kCustomWebUIUrl = "my-webui://show-stats";
+  const char* kCustomUrl = "http://custom.foo.com";
+  const char* kCustomAppUrl = "custom-scheme://custom";
+  const std::vector<std::string> kUrlsThatAlwaysRequireADedicatedProcess = {
+      kExplicitlyIsolatedURL,
+      kUnreachableWebDataURL,
+      GetWebUIURLString("network-error"),
+      kCustomUrl,
+      kCustomAppUrl,
+      kCustomWebUIUrl,
+  };
+
+  CustomBrowserClient modified_client(GURL(kCustomUrl), GURL(kCustomAppUrl),
+                                      /* requires_dedicated_process */ true,
+                                      kCustomWebUIScheme);
+  ContentBrowserClient* regular_client =
+      SetBrowserClientForTesting(&modified_client);
+
+  IsolationContext isolation_context(context());
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  policy->AddIsolatedOrigins(
+      {url::Origin::Create(GURL(kExplicitlyIsolatedURL))},
+      IsolatedOriginSource::TEST);
+
+  for (const auto& url : kUrlsThatAlwaysRequireADedicatedProcess) {
+    EXPECT_TRUE(SiteInstanceImpl::DoesSiteRequireDedicatedProcess(
+        isolation_context, GURL(url)));
+  }
+
+  for (const auto& url : kUrlsThatDoNotRequireADedicatedProcess) {
+    EXPECT_EQ(AreAllSitesIsolatedForTesting(),
+              SiteInstanceImpl::DoesSiteRequireDedicatedProcess(
+                  isolation_context, GURL(url)));
+  }
+  SetBrowserClientForTesting(regular_client);
+}
+
 }  // namespace content