From d02e11a1f3e3b19863567f6287b589799c86391d Mon Sep 17 00:00:00 2001 From: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Wed, 23 Aug 2023 14:00:03 +0900 Subject: [PATCH] Check url's scheme before binding with WebUI (uplift to 1.58.x) (#19810) * Uplift of #19806 (squashed) to beta * Merge pull request #19820 from brave/bsc-crash-follow-up Further restrict the schema for WebUI factory and redirect --------- Co-authored-by: Brian Clifton --- browser/brave_content_browser_client.cc | 13 +++++++++++++ browser/ui/brave_browser_browsertest.cc | 5 +++++ browser/ui/webui/brave_web_ui_controller_factory.cc | 11 +++++++++++ 3 files changed, 29 insertions(+) diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc index 8cc8b9704ebe..39dfbb5ce58b 100644 --- a/browser/brave_content_browser_client.cc +++ b/browser/brave_content_browser_client.cc @@ -1001,6 +1001,18 @@ GURL BraveContentBrowserClient::GetEffectiveURL( bool BraveContentBrowserClient::HandleURLOverrideRewrite( GURL* url, content::BrowserContext* browser_context) { + // Some of these rewrites are for WebUI pages with URL that has moved. + // After rewrite happens, GetWebUIFactoryFunction() will work as expected. + // (see browser\ui\webui\brave_web_ui_controller_factory.cc for more info) + // + // Scope of schema is intentionally narrower than content::HasWebUIScheme(url) + // which also allows both `chrome-untrusted` and `chrome-devtools`. + if (!url->SchemeIs(content::kBraveUIScheme) && + !url->SchemeIs(content::kChromeUIScheme)) { + return false; + } + + // brave://sync => brave://settings/braveSync if (url->host() == chrome::kChromeUISyncHost) { GURL::Replacements replacements; replacements.SetSchemeStr(content::kChromeUIScheme); @@ -1011,6 +1023,7 @@ bool BraveContentBrowserClient::HandleURLOverrideRewrite( } #if !BUILDFLAG(IS_ANDROID) + // brave://adblock => brave://settings/shields/filters if (url->host() == kAdblockHost) { GURL::Replacements replacements; replacements.SetSchemeStr(content::kChromeUIScheme); diff --git a/browser/ui/brave_browser_browsertest.cc b/browser/ui/brave_browser_browsertest.cc index d154ece45401..7d332ae1786c 100644 --- a/browser/ui/brave_browser_browsertest.cc +++ b/browser/ui/brave_browser_browsertest.cc @@ -41,6 +41,11 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserBrowserTest, NTPFaviconTest) { browser()->ShouldDisplayFavicon(tab_model->GetActiveWebContents())); } +IN_PROC_BROWSER_TEST_F(BraveBrowserBrowserTest, LoadWebUIURLWithBadSchemeTest) { + ASSERT_TRUE( + ui_test_utils::NavigateToURL(browser(), GURL("http://settings/"))); +} + IN_PROC_BROWSER_TEST_F(BraveBrowserBrowserTest, OpenNewTabWhenTabStripIsEmpty) { ASSERT_TRUE(embedded_test_server()->Start()); Browser* new_browser = OpenNewBrowser(browser()->profile()); diff --git a/browser/ui/webui/brave_web_ui_controller_factory.cc b/browser/ui/webui/brave_web_ui_controller_factory.cc index 6b766f755263..4d1f710d235d 100644 --- a/browser/ui/webui/brave_web_ui_controller_factory.cc +++ b/browser/ui/webui/brave_web_ui_controller_factory.cc @@ -32,6 +32,7 @@ #include "chrome/common/url_constants.h" #include "components/prefs/pref_service.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/url_utils.h" #include "url/gurl.h" #if !BUILDFLAG(IS_ANDROID) @@ -189,6 +190,16 @@ WebUIController* NewWebUI(WebUI* web_ui, const GURL& url) { WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui, Profile* profile, const GURL& url) { + // This will get called a lot to check all URLs, so do a quick check of other + // schemes to filter out most URLs. + // + // This has a narrow scoper scope than content::HasWebUIScheme(url) which also + // allows both `chrome-untrusted` and `chrome-devtools`. + if (!url.SchemeIs(content::kBraveUIScheme) && + !url.SchemeIs(content::kChromeUIScheme)) { + return nullptr; + } + if (url.host_piece() == kAdblockHost || url.host_piece() == kAdblockInternalsHost || url.host_piece() == kWebcompatReporterHost ||