[Merge M116] webui: Filter out non-chrome scheme URLs in WebUIConfigMap

url::Origin::Create() drops filesystem: and blob: from URLs so filter
those out before using Create().

(cherry picked from commit b46c372)

Bug: b/291154025
Change-Id: Ia8fd0d1048deaef346accd7b4cb64b448e8dc9f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4686653
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1172220}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4719366
Cr-Commit-Position: refs/branch-heads/5845@{#833}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}

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);

chrome/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc

@@ -80,7 +80,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));
     auto create_controller_func = base::BindLambdaForTesting(
         [&](content::WebUI* web_ui,
             const GURL& url) -> std::unique_ptr<content::WebUIController> {

chrome/browser/chrome_content_browser_client.cc

@@ -6681,8 +6681,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;
   }
 

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -1840,7 +1840,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

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);

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<WebUIController> 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<TestConfig>("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<TestConfig>("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<TestConfig>("chrome", "foo"));
+  ScopedWebUIConfigRegistration untrusted_config(
+      std::make_unique<TestConfig>("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

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<WebUIController> 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<WebUIConfig> 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<WebUIConfig> WebUIConfigMap::RemoveConfig(
-    const url::Origin& origin) {
-  auto it = configs_map_.find(origin);
+std::unique_ptr<WebUIConfig> 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;
 

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<WebUIConfig> 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<WebUIConfig> RemoveConfig(const url::Origin& origin);
+  // Removes and returns the WebUIConfig with |url|. Returns nullptr if
+  // there is no WebUIConfig with |url|.
+  std::unique_ptr<WebUIConfig> RemoveConfig(const GURL& url);
 
   // Returns the size of the map, i.e. how many WebUIConfigs are registered.
   size_t GetSizeForTesting() { return configs_map_.size(); }

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<WebUIConfig> webui_config) {
@@ -63,23 +63,23 @@ ScopedWebUIControllerFactoryRegistration::
 
 ScopedWebUIConfigRegistration::ScopedWebUIConfigRegistration(
     std::unique_ptr<WebUIConfig> 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));
 }
 
 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.

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<WebUIConfig> 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<WebUIConfig> replaced_webui_config_;
 };
 

content/test/BUILD.gn

@@ -2686,6 +2686,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",