Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow Shields to block urls with invalid origins (uplift to 0.69.x) #3224

Merged
merged 1 commit into from
Aug 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions browser/extensions/api/brave_shields_api_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ class BraveShieldsAPIBrowserTest : public InProcessBrowserTest {
true);
}

void AllowScriptOriginAndDataURLOnce(const std::string& origin,
const std::string& dataURL) {
scoped_refptr<BraveShieldsAllowScriptsOnceFunction> function(
new BraveShieldsAllowScriptsOnceFunction());
function->set_extension(extension().get());
function->set_has_callback(true);

const GURL url(embedded_test_server()->GetURL(origin, "/simple.js"));
const std::string allow_origin = url.GetOrigin().spec();

int tabId = extensions::ExtensionTabUtil::GetTabId(active_contents());
RunFunctionAndReturnSingleResult(
function.get(),
"[[\"" + allow_origin + "\",\"" + dataURL + "\"], " +
std::to_string(tabId) + "]",
browser());

// reload page with dataURL temporarily allowed
active_contents()->GetController().Reload(content::ReloadType::NORMAL,
true);
}

private:
HostContentSettingsMap* content_settings_;
scoped_refptr<const extensions::Extension> extension_;
Expand Down Expand Up @@ -145,6 +167,28 @@ IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, AllowScriptsOnce) {
"All script loadings should be blocked after navigating away.";
}

IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, AllowScriptsOnceDataURL) {
EXPECT_TRUE(
NavigateToURLUntilLoadStop("a.com", "/load_js_from_origins.html"));
EXPECT_EQ(active_contents()->GetAllFrames().size(), 4u)
<< "All script loadings should not be blocked by default.";

BlockScripts();
EXPECT_TRUE(
NavigateToURLUntilLoadStop("a.com", "/load_js_from_origins.html"));
EXPECT_EQ(active_contents()->GetAllFrames().size(), 1u)
<< "All script loadings should be blocked.";

AllowScriptOriginAndDataURLOnce("a.com",
"data:application/javascript;base64,"
"dmFyIGZyYW1lID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnaWZyYW1lJyk7CmRvY3VtZW"
"50LmJvZHkuYXBwZW5kQ2hpbGQoZnJhbWUpOw==");

EXPECT_TRUE(WaitForLoadStop(active_contents()));
EXPECT_EQ(active_contents()->GetAllFrames().size(), 3u)
<< "Scripts from a.com and data URL should be temporarily allowed.";
}

IN_PROC_BROWSER_TEST_F(BraveShieldsAPIBrowserTest, AllowScriptsOnceIframe) {
BlockScripts();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { NoScriptInfo, NoScriptEntry } from '../types/other/noScriptInfo'
import { getLocale } from '../background/api/localeAPI'

// Helpers
import { getOrigin } from './urlUtils'
import { getOrigin, isHttpOrHttps } from './urlUtils'

/**
* Filter resources by origin to be used for generating NoScriptInfo.
Expand Down Expand Up @@ -121,5 +121,5 @@ export const getBlockScriptText = (haveUserInteracted: boolean, isBlocked: boole
export const getAllowedScriptsOrigins = (modifiedNoScriptInfo: NoScriptInfo): Array<string> => {
const getAllowedOrigins = Object.entries(modifiedNoScriptInfo)
.filter(url => url[1].actuallyBlocked === false)
return getAllowedOrigins.map(url => getOrigin(url[0]) + '/')
return getAllowedOrigins.map(url => isHttpOrHttps(url[0]) ? getOrigin(url[0]) + '/' : url[0])
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,27 @@ export const hasPortNumber = (url: string) => {
* Get the URL origin via Web API
* @param {string} url - The URL to get the origin from
*/
export const getOrigin = (url: string) => new window.URL(url).origin
export const getOrigin = (url: string) => {
// for URLs such as blob:// and data:// that doesn't have a
// valid origin, return the full url.
if (!isHttpOrHttps(url)) {
return url
}
return new window.URL(url).origin
}

/**
* Get the URL hostname via Web API
* @param {string} url - The URL to get the origin from
*/
export const getHostname = (url: string) => new window.URL(url).hostname
export const getHostname = (url: string) => {
// for URLs such as blob:// and data:// that doesn't have a
// valid origin, return the full url.
if (!isHttpOrHttps(url)) {
return url
}
return new window.URL(url).hostname
}

/**
* Strip http/https protocol
Expand Down
8 changes: 8 additions & 0 deletions components/test/brave_extension/helpers/urlUtils_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,20 @@ describe('urlUtils test', () => {
const url = 'https://pokemons-invading-tests-breaking-stuff.com/you-knew-that.js'
expect(getOrigin(url)).toBe('https://pokemons-invading-tests-breaking-stuff.com')
})
it('returns the full URL if origin is not valid', () => {
const url = 'data:application/javascript;base64,c3z4r4vgvst0n00b'
expect(getOrigin(url)).toBe('data:application/javascript;base64,c3z4r4vgvst0n00b')
})
})
describe('getHostname', () => {
it('properly gets the hostname from an URL', () => {
const url = 'https://pokemons-invading-tests-breaking-stuff.com/you-knew-that.js'
expect(getHostname(url)).toBe('pokemons-invading-tests-breaking-stuff.com')
})
it('returns the full URL if origin is not valid', () => {
const url = 'data:application/javascript;base64,c3z4r4vgvst0n00b'
expect(getHostname(url)).toBe('data:application/javascript;base64,c3z4r4vgvst0n00b')
})
})
describe('stripProtocolFromUrl', () => {
it('properly strips out an HTTP protocol', () => {
Expand Down
9 changes: 6 additions & 3 deletions renderer/brave_content_settings_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@ void BraveContentSettingsObserver::DidCommitProvisionalLoad(

bool BraveContentSettingsObserver::IsScriptTemporilyAllowed(
const GURL& script_url) {
// check if scripts from this origin are temporily allowed or not
return base::ContainsKey(temporarily_allowed_scripts_,
script_url.GetOrigin().spec());
// Check if scripts from this origin are temporily allowed or not.
// Also matches the full script URL to support data URL cases which we use
// the full URL to allow it.
return base::ContainsKey(
temporarily_allowed_scripts_, script_url.GetOrigin().spec()) ||
base::ContainsKey(temporarily_allowed_scripts_, script_url.spec());
}

void BraveContentSettingsObserver::BraveSpecificDidBlockJavaScript(
Expand Down
4 changes: 2 additions & 2 deletions renderer/brave_content_settings_observer_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, AllowScripts) {

EXPECT_TRUE(
NavigateToURLUntilLoadStop("a.com", "/load_js_from_origins.html"));
EXPECT_EQ(contents()->GetAllFrames().size(), 3u);
EXPECT_EQ(contents()->GetAllFrames().size(), 4u);
}

IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest,
Expand All @@ -747,7 +747,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest,

EXPECT_TRUE(
NavigateToURLUntilLoadStop("a.com", "/load_js_from_origins.html"));
EXPECT_EQ(contents()->GetAllFrames().size(), 3u);
EXPECT_EQ(contents()->GetAllFrames().size(), 4u);
}

IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest,
Expand Down
1 change: 1 addition & 0 deletions test/data/load_js_from_origins.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
See comments here <https://cs.chromium.org/chromium/src/content/public/test/browser_test_utils.h?l=434>
for more details on how CrossSiteRedirector works.
-->
<script src="data:application/javascript;base64,dmFyIGZyYW1lID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnaWZyYW1lJyk7CmRvY3VtZW50LmJvZHkuYXBwZW5kQ2hpbGQoZnJhbWUpOw=="></script>
<script src="/cross-site/a.com/create_iframe.js"></script>
<script src="/cross-site/b.com/create_iframe.js"></script>
</body></html>