From 1c7141ad185b2156e702499a5135df01c0b04f51 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 14 Oct 2019 12:06:32 +0200 Subject: [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 Reviewed-by: Alex Moshchuk Cr-Original-Commit-Position: refs/heads/master@{#689561} Reviewed-by: Aaron Colwell Cr-Commit-Position: refs/branch-heads/3865@{#595} Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094} Reviewed-by: Michal Klocek --- chromium/content/browser/site_instance_impl.cc | 10 ++-- .../content/browser/site_instance_impl_unittest.cc | 69 ++++++++++++++++++++++ 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* additional_schemes) override { + additional_schemes->push_back(additional_webui_scheme_); + } + + const std::string additional_webui_scheme_; + }; + + const std::vector 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 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 -- cgit v1.2.1