diff --git a/chrome/browser/ash/system_extensions/system_extensions_install_manager.cc b/chrome/browser/ash/system_extensions/system_extensions_install_manager.cc index 801fbc8934a41..19f9cdb7d0d33 100644 --- a/chrome/browser/ash/system_extensions/system_extensions_install_manager.cc +++ b/chrome/browser/ash/system_extensions/system_extensions_install_manager.cc @@ -182,13 +182,12 @@ void SystemExtensionsInstallManager::Uninstall( return; } - const url::Origin& origin = url::Origin::Create(system_extension->base_url); - // Uninstallation Step #1: Unregister the Service Worker. service_worker_manager_->UnregisterServiceWorker(system_extension_id); // Uninstallation Step #2: Remove the WebUIConfig for the System Extension. - content::WebUIConfigMap::GetInstance().RemoveConfig(origin); + content::WebUIConfigMap::GetInstance().RemoveConfig( + system_extension->base_url); // Installation Step #3: Remove the System Extension from persistent storage. persistent_storage_->Remove(system_extension_id); diff --git a/chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc b/chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc index 70e852d798de7..40a367103c338 100644 --- a/chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc +++ b/chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc @@ -76,7 +76,7 @@ class DemoModeAppIntegrationTestBase : public ash::SystemWebAppIntegrationTest { base::ScopedAllowBlockingForTesting allow_blocking; ASSERT_TRUE(component_dir_.CreateUniqueTempDir()); content::WebUIConfigMap::GetInstance().RemoveConfig( - url::Origin::Create(GURL(ash::kChromeUntrustedUIDemoModeAppURL))); + GURL(ash::kChromeUntrustedUIDemoModeAppURL)); content::WebUIConfigMap::GetInstance().AddUntrustedWebUIConfig( std::make_unique( base::BindLambdaForTesting( diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index fbe839fd4259d..f5c614464662c 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -6549,8 +6549,8 @@ bool ChromeContentBrowserClient::HandleWebUI( if (!ChromeWebUIControllerFactory::GetInstance()->UseWebUIForURL( browser_context, *url) && - !content::WebUIConfigMap::GetInstance().GetConfig( - browser_context, url::Origin::Create(*url))) { + !content::WebUIConfigMap::GetInstance().GetConfig(browser_context, + *url)) { return false; } diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc index 1445a22b3470a..36391a685ac6f 100644 --- a/content/browser/service_worker/service_worker_context_wrapper.cc +++ b/content/browser/service_worker/service_worker_context_wrapper.cc @@ -1839,7 +1839,7 @@ ServiceWorkerContextWrapper::GetLoaderFactoryForBrowserInitiatedRequest( context()->loader_factory_bundle_for_update_check()->Clone(); if (auto* config = content::WebUIConfigMap::GetInstance().GetConfig( - browser_context(), scope_origin)) { + browser_context(), scope)) { // If this is a Service Worker for a WebUI, the WebUI's URLDataSource // needs to be registered. Registering a URLDataSource allows the // WebUIURLLoaderFactory below to serve the resources for the WebUI. We diff --git a/content/browser/webui/web_ui_browsertest.cc b/content/browser/webui/web_ui_browsertest.cc index 3c413dc8d23e4..ef391ca763a04 100644 --- a/content/browser/webui/web_ui_browsertest.cc +++ b/content/browser/webui/web_ui_browsertest.cc @@ -211,7 +211,7 @@ IN_PROC_BROWSER_TEST_F(WebUIImplBrowserTest, ForceSwapOnDifferenteWebUITypes) { // has changed and the new process also has WebUI bindings. const GURL web_ui_url2(GetWebUIURL(kChromeUIGpuHost)); EXPECT_TRUE(WebUIConfigMap::GetInstance().GetConfig( - web_contents->GetBrowserContext(), url::Origin::Create(web_ui_url2))); + web_contents->GetBrowserContext(), web_ui_url2)); ASSERT_TRUE(NavigateToURL(web_contents, web_ui_url2)); auto* new_site_instance = web_contents->GetSiteInstance(); EXPECT_NE(orig_site_instance, new_site_instance); diff --git a/content/browser/webui/webui_config_map_unittest.cc b/content/browser/webui/webui_config_map_unittest.cc new file mode 100644 index 0000000000000..1355353c9d717 --- /dev/null +++ b/content/browser/webui/webui_config_map_unittest.cc @@ -0,0 +1,131 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/public/browser/webui_config_map.h" + +#include "content/public/browser/webui_config.h" +#include "content/public/test/scoped_web_ui_controller_factory_registration.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace content { + +class WebUIController; + +namespace { + +constexpr const char kChromeFoo[] = "chrome://foo"; +constexpr const char kChromeBar[] = "chrome://bar"; +constexpr const char kChromeUntrustedFoo[] = "chrome-untrusted://foo"; +constexpr const char kChromeUntrustedBar[] = "chrome-untrusted://bar"; + +// A valid BrowserContext is not needed for these tests. +BrowserContext* kBrowserContext = nullptr; + +class TestConfig : public WebUIConfig { + public: + TestConfig(base::StringPiece scheme, base::StringPiece host) + : WebUIConfig(scheme, host) {} + ~TestConfig() override = default; + + std::unique_ptr CreateWebUIController( + WebUI* web_ui, + const GURL& url) override { + // Unused in these tests. + return nullptr; + } +}; + +} // namespace + +TEST(WebUIConfigTest, AddAndRemoveChromeUrl) { + auto& map = WebUIConfigMap::GetInstance(); + size_t initial_size = map.GetSizeForTesting(); + + auto chrome_config = std::make_unique("chrome", "foo"); + auto* chrome_config_ptr = chrome_config.get(); + map.AddWebUIConfig(std::move(chrome_config)); + + EXPECT_EQ(initial_size + 1, map.GetSizeForTesting()); + + EXPECT_EQ(chrome_config_ptr, + map.GetConfig(kBrowserContext, GURL(kChromeFoo))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeBar))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeUntrustedFoo))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeUntrustedBar))); + + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeBar))); + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeUntrustedFoo))); + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeUntrustedBar))); + + auto removed_config = map.RemoveConfig(GURL(kChromeFoo)); + EXPECT_EQ(removed_config.get(), chrome_config_ptr); + EXPECT_EQ(initial_size, map.GetSizeForTesting()); +} + +TEST(WebUIConfigTest, AddAndRemoteChromeUntrustedUrl) { + auto& map = WebUIConfigMap::GetInstance(); + size_t initial_size = map.GetSizeForTesting(); + + auto untrusted_config = + std::make_unique("chrome-untrusted", "foo"); + auto* untrusted_config_ptr = untrusted_config.get(); + map.AddUntrustedWebUIConfig(std::move(untrusted_config)); + + EXPECT_EQ(initial_size + 1, map.GetSizeForTesting()); + + EXPECT_EQ(untrusted_config_ptr, + map.GetConfig(kBrowserContext, GURL(kChromeUntrustedFoo))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeUntrustedBar))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeFoo))); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, GURL(kChromeBar))); + + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeUntrustedBar))); + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeFoo))); + EXPECT_EQ(nullptr, map.RemoveConfig(GURL(kChromeBar))); + + auto removed_config = map.RemoveConfig(GURL(kChromeUntrustedFoo)); + EXPECT_EQ(removed_config.get(), untrusted_config_ptr); + EXPECT_EQ(initial_size, map.GetSizeForTesting()); +} + +// Regression test for https://crbug.com/1464456. +TEST(WebUIConfigTest, GetAndRemoveNonChromeUrls) { + auto& map = WebUIConfigMap::GetInstance(); + + ScopedWebUIConfigRegistration chrome_config( + std::make_unique("chrome", "foo")); + ScopedWebUIConfigRegistration untrusted_config( + std::make_unique("chrome-untrusted", "foo")); + + // filesystem: URLs + const GURL file_system_chrome("filesystem:chrome://foo/external/file.txt"); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, file_system_chrome)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(file_system_chrome), ""); + + const GURL file_system_chrome_untrusted( + "filesystem:chrome-untrusted://foo/external/file.txt"); + EXPECT_EQ(nullptr, + map.GetConfig(kBrowserContext, file_system_chrome_untrusted)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(file_system_chrome_untrusted), ""); + + // blob: URLs + const GURL blob_chrome("blob:chrome://foo/GUID"); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, blob_chrome)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(blob_chrome), ""); + + const GURL blob_chrome_untrusted("blob:chrome-untrusted://foo/GUID"); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, blob_chrome_untrusted)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(blob_chrome_untrusted), ""); + + // HTTP/HTTPS URLs + const GURL http("http://foo.com"); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, http)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(http), ""); + + const GURL https("https://foo.com"); + EXPECT_EQ(nullptr, map.GetConfig(kBrowserContext, https)); + EXPECT_DEATH_IF_SUPPORTED(map.RemoveConfig(https), ""); +} + +} // namespace content diff --git a/content/public/browser/webui_config_map.cc b/content/public/browser/webui_config_map.cc index 1b7795db2c985..5f3b73d1595c2 100644 --- a/content/public/browser/webui_config_map.cc +++ b/content/public/browser/webui_config_map.cc @@ -29,8 +29,7 @@ class WebUIConfigMapWebUIControllerFactory : public WebUIControllerFactory { WebUI::TypeID GetWebUIType(BrowserContext* browser_context, const GURL& url) override { - auto* config = - config_map_->GetConfig(browser_context, url::Origin::Create(url)); + auto* config = config_map_->GetConfig(browser_context, url); if (!config) return WebUI::kNoWebUI; @@ -39,15 +38,14 @@ class WebUIConfigMapWebUIControllerFactory : public WebUIControllerFactory { bool UseWebUIForURL(BrowserContext* browser_context, const GURL& url) override { - return config_map_->GetConfig(browser_context, url::Origin::Create(url)); + return config_map_->GetConfig(browser_context, url); } std::unique_ptr CreateWebUIControllerForURL( WebUI* web_ui, const GURL& url) override { auto* browser_context = web_ui->GetWebContents()->GetBrowserContext(); - auto* config = - config_map_->GetConfig(browser_context, url::Origin::Create(url)); + auto* config = config_map_->GetConfig(browser_context, url); if (!config) return nullptr; @@ -95,8 +93,17 @@ void WebUIConfigMap::AddWebUIConfigImpl(std::unique_ptr config) { } WebUIConfig* WebUIConfigMap::GetConfig(BrowserContext* browser_context, - const url::Origin& origin) { - auto origin_and_config = configs_map_.find(origin); + const GURL& url) { + // "filesystem:" and "blob:" get dropped by url::Origin::Create() below. We + // don't want navigations to these URLs to have WebUI bindings, e.g. + // chrome.send() or Mojo.bindInterface(), since some WebUIs currently expose + // untrusted content via these schemes. + if (url.scheme() != kChromeUIScheme && + url.scheme() != kChromeUIUntrustedScheme) { + return nullptr; + } + + auto origin_and_config = configs_map_.find(url::Origin::Create(url)); if (origin_and_config == configs_map_.end()) return nullptr; auto& config = origin_and_config->second; @@ -107,9 +114,11 @@ WebUIConfig* WebUIConfigMap::GetConfig(BrowserContext* browser_context, return config.get(); } -std::unique_ptr WebUIConfigMap::RemoveConfig( - const url::Origin& origin) { - auto it = configs_map_.find(origin); +std::unique_ptr WebUIConfigMap::RemoveConfig(const GURL& url) { + CHECK(url.scheme() == kChromeUIScheme || + url.scheme() == kChromeUIUntrustedScheme); + + auto it = configs_map_.find(url::Origin::Create(url)); if (it == configs_map_.end()) return nullptr; diff --git a/content/public/browser/webui_config_map.h b/content/public/browser/webui_config_map.h index 19777632921fc..a80034506bf45 100644 --- a/content/public/browser/webui_config_map.h +++ b/content/public/browser/webui_config_map.h @@ -10,6 +10,8 @@ #include "content/common/content_export.h" +class GURL; + namespace url { class Origin; } @@ -48,14 +50,13 @@ class CONTENT_EXPORT WebUIConfigMap { // readers tell what type of WebUIConfig is being added. void AddUntrustedWebUIConfig(std::unique_ptr config); - // Returns the WebUIConfig for |origin| if it's registered and the WebUI is + // Returns the WebUIConfig for |url| if it's registered and the WebUI is // enabled. (WebUIs can be disabled based on the profile or feature flags.) - WebUIConfig* GetConfig(BrowserContext* browser_context, - const url::Origin& origin); + WebUIConfig* GetConfig(BrowserContext* browser_context, const GURL& url); - // Removes and returns the WebUIConfig with |origin|. Returns nullptr if - // there is no WebUIConfig with |origin|. - std::unique_ptr RemoveConfig(const url::Origin& origin); + // Removes and returns the WebUIConfig with |url|. Returns nullptr if + // there is no WebUIConfig with |url|. + std::unique_ptr RemoveConfig(const GURL& url); // Returns the size of the map, i.e. how many WebUIConfigs are registered. size_t GetSizeForTesting() { return configs_map_.size(); } diff --git a/content/public/test/scoped_web_ui_controller_factory_registration.cc b/content/public/test/scoped_web_ui_controller_factory_registration.cc index 842a314ad4041..d502474856399 100644 --- a/content/public/test/scoped_web_ui_controller_factory_registration.cc +++ b/content/public/test/scoped_web_ui_controller_factory_registration.cc @@ -16,10 +16,10 @@ namespace content { namespace { -url::Origin GetOriginFromConfig(WebUIConfig& webui_config) { - return url::Origin::Create( - GURL(base::StrCat({webui_config.scheme(), url::kStandardSchemeSeparator, - webui_config.host()}))); +GURL GetUrlFromConfig(WebUIConfig& webui_config) { + return GURL( + base::StrCat({webui_config.scheme(), url::kStandardSchemeSeparator, + webui_config.host()})); } void AddWebUIConfig(std::unique_ptr webui_config) { @@ -63,9 +63,9 @@ ScopedWebUIControllerFactoryRegistration:: ScopedWebUIConfigRegistration::ScopedWebUIConfigRegistration( std::unique_ptr webui_config) - : webui_config_origin_(GetOriginFromConfig(*webui_config)) { + : webui_config_url_(GetUrlFromConfig(*webui_config)) { auto& config_map = WebUIConfigMap::GetInstance(); - replaced_webui_config_ = config_map.RemoveConfig(webui_config_origin_); + replaced_webui_config_ = config_map.RemoveConfig(webui_config_url_); DCHECK(webui_config.get() != nullptr); AddWebUIConfig(std::move(webui_config)); @@ -73,13 +73,13 @@ ScopedWebUIConfigRegistration::ScopedWebUIConfigRegistration( ScopedWebUIConfigRegistration::ScopedWebUIConfigRegistration( const GURL& webui_origin) - : webui_config_origin_(url::Origin::Create(webui_origin)) { + : webui_config_url_(webui_origin) { auto& config_map = WebUIConfigMap::GetInstance(); - replaced_webui_config_ = config_map.RemoveConfig(webui_config_origin_); + replaced_webui_config_ = config_map.RemoveConfig(webui_config_url_); } ScopedWebUIConfigRegistration::~ScopedWebUIConfigRegistration() { - WebUIConfigMap::GetInstance().RemoveConfig(webui_config_origin_); + WebUIConfigMap::GetInstance().RemoveConfig(webui_config_url_); // If we replaced a WebUIConfig, re-register it to keep the global state // clean for future tests. diff --git a/content/public/test/scoped_web_ui_controller_factory_registration.h b/content/public/test/scoped_web_ui_controller_factory_registration.h index afa36dc5b1a52..6269fdc9692f5 100644 --- a/content/public/test/scoped_web_ui_controller_factory_registration.h +++ b/content/public/test/scoped_web_ui_controller_factory_registration.h @@ -7,7 +7,7 @@ #include "base/memory/raw_ptr.h" #include "testing/gtest/include/gtest/gtest.h" -#include "url/origin.h" +#include "url/gurl.h" namespace content { @@ -45,14 +45,14 @@ class ScopedWebUIConfigRegistration { explicit ScopedWebUIConfigRegistration( std::unique_ptr webui_config); - // Removes the WebUIConfig with `webui_origin`. The removed WebUIConfig is + // Removes the WebUIConfig with `webui_url`. The removed WebUIConfig is // re-registered on destruction. - explicit ScopedWebUIConfigRegistration(const GURL& webui_origin); + explicit ScopedWebUIConfigRegistration(const GURL& webui_url); ~ScopedWebUIConfigRegistration(); private: - const url::Origin webui_config_origin_; + const GURL webui_config_url_; std::unique_ptr replaced_webui_config_; }; diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index f70f6a45b30a5..9617f1b06c449 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -2660,6 +2660,7 @@ test("content_unittests") { "../browser/webui/web_ui_unittest.cc", "../browser/webui/web_ui_url_loader_factory_unittest.cc", "../browser/webui/web_ui_webui_js_bridge_unittest.cc", + "../browser/webui/webui_config_map_unittest.cc", "../browser/worker_host/dedicated_worker_service_impl_unittest.cc", "../browser/worker_host/mock_shared_worker.cc", "../browser/worker_host/mock_shared_worker.h",