diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-10-14 12:06:32 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-10-14 12:32:30 +0000 |
commit | 1c7141ad185b2156e702499a5135df01c0b04f51 (patch) | |
tree | e703bd3a96108f4dd23775f1b953db1c024c3e15 | |
parent | bebd1df6d51d5c011d928a25651c5c44d53a4750 (diff) | |
download | qtwebengine-chromium-1c7141ad185b2156e702499a5135df01c0b04f51.tar.gz |
[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>
-rw-r--r-- | chromium/content/browser/site_instance_impl.cc | 10 | ||||
-rw-r--r-- | chromium/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<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 |