brave scheme is loaded from js #2777

simonhong · 2019-01-02T03:34:56Z

Description

brave scheme should be blocked from js. (Not allowed to load local resources)
With below code by @bbondy , our settings page is loaded.
This loading should be blocked.

<html>
<head>
  <script>
    function gotosettings(){
      x = window.open("https://example.com");
      setTimeout(function(){x.location.replace("brave://settings")},2500);
    }
  </script>
</head>
<body>
  <a href="javascript:gotosettings()">Go to brave://settings</a>
</body>
</html>

Steps to Reproduce

Navigate to https://world_languages.gitlab.io/bravepoc/
Click link

Actual result:

brave settings page is loaded

Expected result:

settings page should not be loaded

Reproduces how often:

Brave version (brave://version info)

Version 0.61.0 Chromium: 72.0.3626.28 (Official Build) (64-bit)

Reproducible on current release:

Does it reproduce on brave-browser dev/beta builds?
yes

Website problems only:

Does the issue resolve itself when disabling Brave Shields? No
Is the issue reproducible on the latest version of Chrome? No

Additional Information

The text was updated successfully, but these errors were encountered:

simonhong · 2019-01-02T03:36:32Z

Chrome scheme is added to not allowing list by below.

void RenderThreadImpl::RegisterSchemes() {
  // chrome:
  WebString chrome_scheme(WebString::FromASCII(kChromeUIScheme));
  WebSecurityPolicy::RegisterURLSchemeAsDisplayIsolated(chrome_scheme);
  WebSecurityPolicy::RegisterURLSchemeAsNotAllowingJavascriptURLs(
      chrome_scheme);
  ...
}

When comments out above registering for chrome scheme, chrome scheme page wasn't loaded.
Instead, about:blank#blocked is loaded. It seems there are additional policies for this blocking.
ProfileIOData::IsHandledProtocol() returns true for chrome scheme. If not, ChildProcessSecurityPolicyImpl::CanRequestURL() returns false and then about:blank#blocked is rendered.

simonhong · 2019-01-04T04:49:25Z

Need to know navigation code path(browser-initiated, renderer-initiated) to find good solution.

Does this can be fixed in browser side only? or both?
The reason of this issue is late timing of mapping time?
Adjusting mapping timing can fix this issue? If so how? If not why?
Need to add tests to have good coverage

simonhong · 2019-01-07T15:57:44Z

With Plz navigation, all navigations are managed by browser process.

Also there are two kinds of navigations.

The one is browser-initiated navigation and the other is renderer-initiated navigation.

Although both navigation have different origin(browser or renderer), their navigation is
managed only by browser process.

Below is the case of renderer-initiated navigation

<a> link click
changing window.location.href
redirect via the tag
using window.history.pushState

Below is the case of browser-initiated navigation

any navigation initiated from the omnibox
navigations via suggestions in browser UI
navigations via browser UI: Ctrl-R, refresh/forward/back/home buttons
using window.history.forward() or window.history.back()
any other "explicit" URL navigations, e.g. bookmarks

Although chromium's navigation code path is quite complext, I found good point to distinguish
whether current code path is renderer or browser initiated navigation.
They are NavigationRequest::CreateBrowserInitiated() and NavigationRequest::CreateRendererInitiated()
Regardless of browser/renderer initiated navigation, browser process creates and starts navigation by using above method.

To prevent brave scheme loading from different origin, we should handle it from browser and rendere both.
Currently, we are using url mapping from brave:// to chrome:// in overridden browser_about_handler.cc.

When chrome url is initiated from renderer, its loading is blocked in renderer process. That means current mapping can't prevent renderer initiated brave scheme navigation. If we want to handle like chrome scheme, renderer should prevent brave
scheme. With below code, we can also block brave scheme in renderer process.

void RenderThreadImpl::RegisterSchemes() {
  WebString brave_scheme(WebString::FromASCII(kBraveUIScheme));
  WebSecurityPolicy::RegisterURLSchemeAsDisplayIsolated(brave_scheme);
  WebSecurityPolicy::RegisterURLSchemeAsNotAllowingJavascriptURLs(
      brave_scheme);
  
  ...

}

With above registration, SecurityOrigin::CanDisplay(const KURL& url) can prevent brave scheme navigation from renderer.

In case of browser initiated navigation, current mapping works well because mapping is done before the start of navigation.

In the browser process, it also has filtering policy to check renderer requests like open url.
For that, browser process filters that url by using RenderProcessHostImpl::FilterURL(bool empty_allowed, GURL* url).
Other navigation like Open in new tab from context menu and middle click is checked by this filtering.
To filtering brave scheme by above filter method, brave scheme should be added like below.

bool ProfileIOData::IsHandledProtocol(const std::string& scheme) {
  ...

     extensions::kExtensionScheme,
#endif
     content::kChromeUIScheme,
#if defined(BRAVE_CHROMIUM_BUILD)
    kBraveUIScheme,
#endif
     url::kDataScheme,
 #if defined(OS_CHROMEOS)
     content::kExternalFileScheme,

     ...

}

Remained issues

Grant allowing for loading brave page from brave page
brave page isn't opened from brave page because loaded brave page is mapped to chrome scheme. loading brave webui is prevented from chrome webui page. Brave webui is only opened/loaded from brave webui.
There are two points that grant allowing loading brave webui from brave webui.
For chrome://XXX scheme webui, ChildProcessSecurityPolicyImpl::GrantWebUIBindings() does this in browser-side.
It adds this scheme to granted scheme list, so ChildProcessSecurityPolicyImpl::CanRequestURL() returns true to that scheme. Regardless of adjusting mapping timing, we should handle this for brave scheme because this granting is done during the navigation.
In renderer-side, this granting should be done, too. If not, brave link would not work in brave webui.
Maybe SecurityPolicy::AddOriginAccessAllowListEntry() does that.
unmapped url is checked
1. IsRendererDebugURL(params.url).
  During the browser-initiated navigation, NavigationControllerImpl::NavigateWithoutEntry() checks requested url is for debug by using IsRendererDebugURL(params.url). At this point, brave scheme is not handled properly because params.url is original url not the mapped url to chrome. After that, mapping is done during the creating navigation request.
2. IsURLAllowedInIncognito()
  Some webuis are not allowed to load in private window. That is checked by IsURLAllowedInIncognito(...) before starting navigation at some points. Because of this, many brave webui is loaded in private window.

Fixing ideas

IMO, using chrome scheme for brave page link in brave page is easiest way like <a href="chrome://sync" target="_blank">brave://sync</a> in brave://settings page. This resolves in renderer-side blocking. In browser, we should add handling brave scheme in ChildProcessSecurityPolicyImpl::GrantWebUIBindings(). If not, we should control brave scheme rule for chrome webui or introducing new mapping in renderer?
Need to fine the timing of earlier mapping. Not sure it is possible but do mapping when navigation/loading params are created.

diracdeltas · 2019-01-08T21:03:54Z

just noting there is a related proof-of-concept which should be fixed by this:

<button id="p1">click here to navigate</button>
<script>
var p2;
p1.onclick=e=>{
p2=open("brave://about","","noopener");
}
p1.click();
</script>

simonhong · 2019-01-09T00:20:40Z

@diracdeltas Yep, above one also fixed by #2861

simonhong · 2019-01-11T00:40:27Z

browser-initiated navigation path

Typing in omnibox

content::NavigationRequest::CreateBrowserInitiated()
content::NavigationControllerImpl::CreateNavigationRequestFromLoadParams() 
content::NavigationControllerImpl::NavigateWithoutEntry()
content::NavigationControllerImpl::LoadURLWithParams()
`anonymous namespace'::LoadURLInContents()
chrome::OpenCurrentURL()
chrome::BrowserCommandController::ExecuteCommandWithDisposition()

Back/Forward

In this case, already mapped url is used.

Bookmark

content::NavigationRequest::CreateBrowserInitiated()
content::NavigationControllerImpl::CreateNavigationRequestFromLoadParams()
content::NavigationControllerImpl::NavigateWithoutEntry()
NavigationControllerImpl::LoadURLWithParams()
`anonymous namespace'::LoadURLInContents()
Navigate()
Browser::OpenURLFromTab()
WebContentsImpl::OpenURL()

Newtab button

content.dll!content::NavigationRequest::CreateBrowserInitiated()
content::NavigationControllerImpl::CreateNavigationRequestFromLoadParams()
content::NavigationControllerImpl::NavigateWithoutEntry()
content::NavigationControllerImpl::LoadURLWithParams()
`anonymous namespace'::LoadURLInContents()
Navigate()
chrome::AddTabAt()

New Window

content.dll!content::NavigationRequest::CreateBrowserInitiated()
content::NavigationControllerImpl::CreateNavigationRequestFromLoadParams()
content::NavigationControllerImpl::NavigateWithoutEntry()
content::NavigationControllerImpl::LoadURLWithParams()
'anonymous namespace'::LoadURLInContents()
Navigate()
chrome::AddTabAt()
chrome::OpenEmptyWindow()
chrome::NewEmptyWindow()
chrome::NewWindow()

simonhong · 2019-01-11T00:41:45Z

kChromeUIScheme in content layer

/Users/simon/Projects/brave/brave-browser/src/content/browser/browser_url_handler_impl.cc
32,9: kChromeUIScheme,

/Users/simon/Projects/brave/brave-browser/src/content/browser/code_cache/generated_code_cache.cc
33,42: origin_lock.SchemeIs(content::kChromeUIScheme)) &&

/Users/simon/Projects/brave/brave-browser/src/content/browser/download/download_manager_impl.cc
1245,46: } else if (params->url().SchemeIs(content::kChromeUIScheme)) {

/Users/simon/Projects/brave/brave-browser/src/content/browser/frame_host/debug_urls.cc
72,40: if (!(url.is_valid() && url.SchemeIs(kChromeUIScheme) &&

/Users/simon/Projects/brave/brave-browser/src/content/browser/frame_host/webui_navigation_throttle.cc
24,46: if (navigation_handle()->GetURL().SchemeIs(kChromeUIScheme))
52,46: parent->GetLastCommittedURL().SchemeIs(kChromeUIScheme)) {

/Users/simon/Projects/brave/brave-browser/src/content/browser/net/view_blob_internals_job_factory.cc
17,23: return url.SchemeIs(kChromeUIScheme) &&

/Users/simon/Projects/brave/brave-browser/src/content/browser/renderer_host/code_cache_host_impl.cc
88,37: origin_lock.SchemeIs(content::kChromeUIScheme)) {

/Users/simon/Projects/brave/brave-browser/src/content/browser/site_instance_impl.cc
582,14: // Isolate kChromeUIScheme pages from one another and from other kinds of
584,34: if (site_url.SchemeIs(content::kChromeUIScheme))

/Users/simon/Projects/brave/brave-browser/src/content/browser/webrtc/webrtc_internals_message_handler.cc
73,23: if (!url.SchemeIs(kChromeUIScheme) ||

/Users/simon/Projects/brave/brave-browser/src/content/browser/webui/content_web_ui_controller_factory.cc
28,21: if (!url.SchemeIs(kChromeUIScheme))
62,21: if (!url.SchemeIs(kChromeUIScheme))

/Users/simon/Projects/brave/brave-browser/src/content/browser/webui/shared_resources_data_source.cc
296,39: std::string allowed_origin_prefix = kChromeUIScheme;

/Users/simon/Projects/brave/brave-browser/src/content/browser/webui/url_data_manager_backend.cc
380,33: if (request->url().SchemeIs(kChromeUIScheme) &&
398,33: if (request->url().SchemeIs(kChromeUIScheme) &&
644,23: DCHECK(url.SchemeIs(kChromeUIScheme) ||
694,21: schemes.push_back(kChromeUIScheme);

/Users/simon/Projects/brave/brave-browser/src/content/common/url_schemes.cc
28,3: kChromeUIScheme,
59,26: url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITH_HOST);
69,36: schemes.secure_schemes.push_back(kChromeUIScheme);
81,42: schemes.cors_enabled_schemes.push_back(kChromeUIScheme);

/Users/simon/Projects/brave/brave-browser/src/content/public/browser/url_data_source.cc
71,62: return url.SchemeIs(kChromeDevToolsScheme) || url.SchemeIs(kChromeUIScheme);

/Users/simon/Projects/brave/brave-browser/src/content/public/common/url_constants.cc
14,12: const char kChromeUIScheme[] = "chrome";

/Users/simon/Projects/brave/brave-browser/src/content/public/common/url_utils.cc
24,23: url.SchemeIs(kChromeUIScheme);
77,21: if (!url.SchemeIs(kChromeUIScheme))

/Users/simon/Projects/brave/brave-browser/src/content/renderer/web_ui_extension.cc
47,27: (frame_url.SchemeIs(kChromeUIScheme) ||

simonhong · 2019-01-16T10:25:39Z

Documents about current navigation path - PlzNavigation

tildelowengrimm · 2019-07-23T20:28:22Z

Hey @simonhong — is this fixed or is there still more work left to do?

WorldLanguages · 2019-07-23T23:14:33Z

@tomlowenthal Hello, this issue is the same as #2631, which I reported in HackerOne. This can be closed! :)

simonhong · 2019-07-23T23:21:17Z

@tomlowenthal Yes, this is fixed. brave:// can't be loaded from js.

simonhong self-assigned this Jan 2, 2019

simonhong added the security label Jan 2, 2019

NejcZdovc added this to the 1.x Backlog milestone Jan 2, 2019

simonhong mentioned this issue Jan 2, 2019

Prevent loading brave scheme url from javascript brave/brave-core#1196

Merged

18 tasks

simonhong added the QA/Yes label Jan 2, 2019

bbondy modified the milestones: 1.x Backlog, 0.58.x - Release Channel - Hotfix 2 Jan 2, 2019

bbondy mentioned this issue Jan 8, 2019

Temporary workaround for "brave scheme is loaded from js" #2861

Closed

bbondy modified the milestones: 0.58.x - Release Channel - Hotfix 2, 1.x Backlog Jan 8, 2019

simonhong mentioned this issue Jan 11, 2019

Fix some webui that doesn't allowed in private window is loaded in private window brave/brave-core#1295

Merged

18 tasks

diracdeltas added the priority/P2 A bad problem. We might uplift this to the next planned release. label Jan 15, 2019

rebron removed this from the 1.x Backlog milestone Feb 7, 2019

simonhong closed this as completed Jul 23, 2019

bbondy added this to the Closed / Invalid milestone Jun 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

brave scheme is loaded from js #2777

brave scheme is loaded from js #2777

simonhong commented Jan 2, 2019 •

edited

Loading

simonhong commented Jan 2, 2019 •

edited

Loading

simonhong commented Jan 4, 2019 •

edited

Loading

simonhong commented Jan 7, 2019 •

edited

Loading

diracdeltas commented Jan 8, 2019

simonhong commented Jan 9, 2019

simonhong commented Jan 11, 2019 •

edited

Loading

simonhong commented Jan 11, 2019

simonhong commented Jan 16, 2019 •

edited

Loading

tildelowengrimm commented Jul 23, 2019

WorldLanguages commented Jul 23, 2019

simonhong commented Jul 23, 2019

brave scheme is loaded from js #2777

brave scheme is loaded from js #2777

Comments

simonhong commented Jan 2, 2019 • edited Loading

Description

Steps to Reproduce

Actual result:

Expected result:

Reproduces how often:

Brave version (brave://version info)

Reproducible on current release:

Website problems only:

Additional Information

simonhong commented Jan 2, 2019 • edited Loading

simonhong commented Jan 4, 2019 • edited Loading

simonhong commented Jan 7, 2019 • edited Loading

With Plz navigation, all navigations are managed by browser process.

Remained issues

Fixing ideas

diracdeltas commented Jan 8, 2019

simonhong commented Jan 9, 2019

simonhong commented Jan 11, 2019 • edited Loading

browser-initiated navigation path

simonhong commented Jan 11, 2019

kChromeUIScheme in content layer

simonhong commented Jan 16, 2019 • edited Loading

tildelowengrimm commented Jul 23, 2019

WorldLanguages commented Jul 23, 2019

simonhong commented Jul 23, 2019

simonhong commented Jan 2, 2019 •

edited

Loading

simonhong commented Jan 2, 2019 •

edited

Loading

simonhong commented Jan 4, 2019 •

edited

Loading

simonhong commented Jan 7, 2019 •

edited

Loading

simonhong commented Jan 11, 2019 •

edited

Loading

simonhong commented Jan 16, 2019 •

edited

Loading