Skip to content

Commit

Permalink
Check url's scheme before binding with WebUI (uplift to 1.58.x) (#19810)
Browse files Browse the repository at this point in the history
* 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 <brian@clifton.me>
  • Loading branch information
brave-builds and bsclifton authored Aug 23, 2023
1 parent 71ede89 commit d02e11a
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 0 deletions.
13 changes: 13 additions & 0 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions browser/ui/brave_browser_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
11 changes: 11 additions & 0 deletions browser/ui/webui/brave_web_ui_controller_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 ||
Expand Down

0 comments on commit d02e11a

Please sign in to comment.