Restrict [un]registerProtocolHandler to secure contexts
The registerProtocolHandler() and unregisterProtocolHandler() functions
should only be permitted for secure contexts. When called on non-secure
contexts, a SecurityError should be thrown.
Bug: 882284
Change-Id: Iacf3d31f80f5118e9e9aacad2c99a0111d6e7cc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1892213
Reviewed-by: Gyuyoung Kim <gyuyoung@igalia.com>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Eric Lawrence [MSFT] <ericlaw@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#717501}
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 4cae576..dcd6c324 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -1290,6 +1290,7 @@
// Checks that registering a protocol handler causes cancellation.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderRegisterProtocolHandler) {
+ UseHttpsSrcServer();
PrerenderTestURL("/prerender/prerender_register_protocol_handler.html",
FINAL_STATUS_REGISTER_PROTOCOL_HANDLER, 0);
}
diff --git a/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.cc b/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.cc
index 737cb4d..3be6ca96 100644
--- a/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.cc
+++ b/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.cc
@@ -80,13 +80,14 @@
return false;
}
- // Although not enforced in the spec the spec gives freedom to do additional
- // security checks. Bugs have arisen from allowing non-http/https URLs, e.g.
+ // Although not required by the spec, the spec allows additional security
+ // checks. Bugs have arisen from allowing non-http/https URLs, e.g.
// https://crbug.com/971917 and it doesn't make a lot of sense to support
- // them. We also need to allow extensions to continue using the API.
+ // them. We do need to allow extensions to continue using the API.
if (!kurl.ProtocolIsInHTTPFamily() && !kurl.ProtocolIs("chrome-extension")) {
exception_state.ThrowSecurityError(
- "The scheme of the url provided must be the 'http' or 'https'.");
+ "The scheme of the url provided must be 'https' or "
+ "'chrome-extension'.");
return false;
}
@@ -104,18 +105,19 @@
static bool VerifyCustomHandlerScheme(const String& scheme,
ExceptionState& exception_state) {
if (!IsValidProtocol(scheme)) {
- exception_state.ThrowSecurityError("The scheme '" + scheme +
- "' is not valid protocol");
+ exception_state.ThrowSecurityError(
+ "The scheme name '" + scheme +
+ "' is not allowed by URI syntax (RFC3986).");
return false;
}
if (scheme.StartsWith("web+")) {
// The specification requires that the length of scheme is at least five
- // characteres (including 'web+' prefix).
+ // characters (including 'web+' prefix).
if (scheme.length() >= 5)
return true;
- exception_state.ThrowSecurityError("The scheme '" + scheme +
+ exception_state.ThrowSecurityError("The scheme name '" + scheme +
"' is less than five characters long.");
return false;
}
@@ -125,8 +127,8 @@
exception_state.ThrowSecurityError(
"The scheme '" + scheme +
- "' doesn't belong to the scheme whitelist. "
- "Please prefix non-whitelisted schemes "
+ "' doesn't belong to the scheme allowlist. "
+ "Please prefix non-allowlisted schemes "
"with the string 'web+'.");
return false;
}
@@ -154,14 +156,15 @@
LocalFrame* frame = navigator.GetFrame();
if (!frame)
return;
-
Document* document = frame->GetDocument();
DCHECK(document);
- if (!VerifyCustomHandlerURL(*document, url, exception_state))
+ // Per the HTML specification, exceptions for arguments must be surfaced in
+ // the order of the arguments.
+ if (!VerifyCustomHandlerScheme(scheme, exception_state))
return;
- if (!VerifyCustomHandlerScheme(scheme, exception_state))
+ if (!VerifyCustomHandlerURL(*document, url, exception_state))
return;
// Count usage; perhaps we can forbid this from cross-origin subframes as
@@ -170,7 +173,8 @@
*document, frame->IsCrossOriginSubframe()
? WebFeature::kRegisterProtocolHandlerCrossOriginSubframe
: WebFeature::kRegisterProtocolHandlerSameOriginAsTop);
- // Count usage; perhaps we can lock this to secure contexts.
+ // Count usage. Context should now always be secure due to the same-origin
+ // check and the requirement that the calling context be secure.
UseCounter::Count(*document,
document->IsSecureContext()
? WebFeature::kRegisterProtocolHandlerSecureOrigin
@@ -192,10 +196,10 @@
Document* document = frame->GetDocument();
DCHECK(document);
- if (!VerifyCustomHandlerURL(*document, url, exception_state))
+ if (!VerifyCustomHandlerScheme(scheme, exception_state))
return;
- if (!VerifyCustomHandlerScheme(scheme, exception_state))
+ if (!VerifyCustomHandlerURL(*document, url, exception_state))
return;
NavigatorContentUtils::From(navigator, *frame)
diff --git a/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.idl b/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.idl
index d36a5f4b..8c570f7 100644
--- a/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.idl
+++ b/third_party/blink/renderer/modules/navigatorcontentutils/navigator_content_utils.idl
@@ -23,6 +23,6 @@
[
ImplementedAs=NavigatorContentUtils
] partial interface Navigator {
- [RuntimeEnabled=NavigatorContentUtils, RaisesException] void registerProtocolHandler(DOMString scheme, USVString url, DOMString title);
- [RuntimeEnabled=NavigatorContentUtils, RaisesException] void unregisterProtocolHandler(DOMString scheme, USVString url);
+ [RuntimeEnabled=NavigatorContentUtils, RaisesException, SecureContext] void registerProtocolHandler(DOMString scheme, USVString url, DOMString title);
+ [RuntimeEnabled=NavigatorContentUtils, RaisesException, SecureContext] void unregisterProtocolHandler(DOMString scheme, USVString url);
};
diff --git a/third_party/blink/web_tests/external/wpt/html/dom/usvstring-reflection.https.html b/third_party/blink/web_tests/external/wpt/html/dom/usvstring-reflection.https.html
index b9cafd1..162d58c 100644
--- a/third_party/blink/web_tests/external/wpt/html/dom/usvstring-reflection.https.html
+++ b/third_party/blink/web_tests/external/wpt/html/dom/usvstring-reflection.https.html
@@ -107,12 +107,12 @@
test(() => {
// This shouldn't throw an exception.
window.navigator.registerProtocolHandler('web+myprotocol', "custom-scheme\uD800/url=%s", "title");
-}, "RegisterPtotocolHandler URL: unpaired surrogate codepoint should not make any exceptions.")
+}, "RegisterProtocolHandler URL: unpaired surrogate codepoint should not make any exceptions.")
test(() => {
// This shouldn't throw an exception.
window.navigator.unregisterProtocolHandler('web+myprotocol', "custom-scheme\uD800/url=%s");
-}, "UnregisterPtotocolHandler URL: unpaired surrogate codepoint should not make any exceptions.")
+}, "UnregisterProtocolHandler URL: unpaired surrogate codepoint should not make any exceptions.")
test(() => {
var w = window.open("about:blank#\uD800");
diff --git a/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/protocol.https.html b/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/protocol.https.html
index 006827f1..dcf1007 100644
--- a/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/protocol.https.html
+++ b/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/protocol.https.html
@@ -21,7 +21,7 @@
}, 'the unregisterProtocolHandler method should exist on the navigator object');
/* URL argument */
-const vaild_urls = [
+const valid_urls = [
'%s',
location.href + '/%s',
location.href + '#%s',
@@ -31,7 +31,7 @@
location.href + '/%s/bar/baz/?foo=1337&bar#baz',
location.href + '/%s/foo/%s/',
];
-for (const url of vaild_urls) {
+for (const url of valid_urls) {
test(() => {
navigator.registerProtocolHandler('tel', url, 'foo');
}, 'registerProtocolHandler: Valid URL "' + url + '" should work.');
@@ -61,7 +61,7 @@
}, 'unregisterProtocolHandler: Invalid URL "' + url + '" should throw SYNTAX_ERR.');
}
-const invaild_urls2 = [
+const invalid_urls2 = [
'http://%s.com',
'http://%s.example.com',
'http://example.com/%s',
@@ -70,7 +70,7 @@
'mailto:%s@example.com',
'mailto:%s',
];
-for (const url of invaild_urls2) {
+for (const url of invalid_urls2) {
test(() => {
assert_throws('SECURITY_ERR', () => { navigator.registerProtocolHandler('mailto', url, 'foo'); });
}, 'registerProtocolHandler: Invalid URL "' + url + '" should throw SECURITY_ERR.');
@@ -84,7 +84,7 @@
/* Overriding any of the following protocols must never be allowed. That would
* break the browser. */
-const blacklist = [
+const denylist = [
'about',
'attachment',
'blob',
@@ -124,13 +124,13 @@
'tel:sip',
'web+',
];
-for (const scheme of blacklist) {
+for (const scheme of denylist) {
test(() => {
assert_throws('SECURITY_ERR', () => { navigator.registerProtocolHandler(scheme, location.href + '/%s', 'foo'); });
}, 'registerProtocolHandler: Attempting to override the "' + scheme + '" protocol should throw SECURITY_ERR.');
test(() => {
- assert_throws('SECURITY_ERR', () => { navigator.unregisterProtocolHandler(scheme, location.href + '/%s', 'foo'); });
+ assert_throws('SECURITY_ERR', () => { navigator.unregisterProtocolHandler(scheme, location.href + '/%s'); });
}, 'unregisterProtocolHandler: Attempting to override the "' + scheme + '" protocol should throw SECURITY_ERR.');
}
@@ -161,7 +161,7 @@
'webcal',
'wtai',
'xmpp',
- /*other vaild schemes*/
+ /*other valid schemes*/
'BitcoIn',
'Irc',
'MagneT',
diff --git a/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/secure_context.html b/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/secure_context.html
new file mode 100644
index 0000000..685f5d1
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/html/webappapis/system-state-and-capabilities/the-navigator-object/secure_context.html
@@ -0,0 +1,18 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <script>
+ test(t => {
+ assert_false('registerProtocolHandler' in navigator);
+ assert_equals(navigator.registerProtocolHandler, undefined);
+ }, "navigator.registerProtocolHandler does not exist in non-secure contexts.");
+
+ test(t => {
+ assert_false('unregisterProtocolHandler' in navigator);
+ assert_equals(navigator.unregisterProtocolHandler, undefined);
+ }, "navigator.unregisterProtocolHandler does not exist in non-secure contexts.");
+ </script>
+</head>
+</html>
diff --git a/third_party/blink/web_tests/register-protocol-handler/blob-urls.html b/third_party/blink/web_tests/register-protocol-handler/blob-urls.html
index 9bee82d..c528464 100644
--- a/third_party/blink/web_tests/register-protocol-handler/blob-urls.html
+++ b/third_party/blink/web_tests/register-protocol-handler/blob-urls.html
@@ -3,10 +3,9 @@
<script src="../resources/testharnessreport.js"></script>
<title>Test that blob: URLs fail registerProtocolHandler checks</title>
<body>
-<iframe id='frame'></iframe>
<script>
-const frameSrc =
+const winSrc =
`<html>
<head>
<script>
@@ -14,9 +13,9 @@
navigator.registerProtocolHandler('web+test',
location.href + '?%s',
'Test handler');
- window.parent.postMessage('rph succeeded', '*');
+ window.opener.postMessage('rph succeeded', '*');
} catch (e) {
- window.parent.postMessage('rph failed: ' + e.message, '*');
+ window.opener.postMessage('rph failed: ' + e.message, '*');
}
<\/script>
<\/head>
@@ -28,12 +27,13 @@
window.addEventListener('message', (msg) => {
assert_equals(msg.data,
- 'rph failed: Failed to execute \'registerProtocolHandler\' on \'Navigator\': The scheme of the url provided must be the \'http\' or \'https\'.',
+ 'rph failed: Failed to execute \'registerProtocolHandler\' on \'Navigator\': The scheme of the url provided must be \'https\' or \'chrome-extension\'.',
'registerProtocolHandler should have failed');
done();
});
-const data = new Blob([frameSrc], {type: 'text/html'});
-document.getElementById('frame').src = window.URL.createObjectURL(data);
+const data = new Blob([winSrc], {type: 'text/html'});
+const objectURL = window.URL.createObjectURL(data);
+window.open(objectURL, '_blank');
</script>
</body>
diff --git a/third_party/blink/web_tests/register-protocol-handler/data-urls.html b/third_party/blink/web_tests/register-protocol-handler/data-urls.https.html
similarity index 78%
rename from third_party/blink/web_tests/register-protocol-handler/data-urls.html
rename to third_party/blink/web_tests/register-protocol-handler/data-urls.https.html
index 89bde5f..2c2c5a3c 100644
--- a/third_party/blink/web_tests/register-protocol-handler/data-urls.html
+++ b/third_party/blink/web_tests/register-protocol-handler/data-urls.https.html
@@ -28,8 +28,7 @@
window.addEventListener('message', (msg) => {
assert_equals(msg.data,
- 'rph failed: Failed to execute \'registerProtocolHandler\' on \'Navigator\': The scheme of the url provided must be the \'http\' or \'https\'.',
- 'registerProtocolHandler should have failed');
+ 'rph failed: navigator.registerProtocolHandler is not a function');
done();
});
