Commit 27205ad8 authored by Fabio Rocha's avatar Fabio Rocha Committed by Commit Bot

desktop-pwas: Refactor navigator_content_utils validation functions

Manifest-based registration for protocol handlers should, as much as
possible, use the same validation logic used by
`registerProtocolHandler`.

In order to do that we needed to modify NavigatorContentUtils a bit
for some of the validation functions not be dependent on objects not
known to the manifest parser (e.g.: Document, ExceptionState..).
Instead, we extract some error handling logic out and pass arguments
in the format ready to be validated.

A subsequent CL will include modifications for the manifest parser
to consume from the functions exposed here.

Bug: 1019239
Change-Id: I38189e305b67a4ccb9820cbb450546e056774c97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2227469Reviewed-by: default avatarLorne Mitchell <lomitch@microsoft.com>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Reviewed-by: default avatarGyuyoung Kim <gyuyoung@igalia.com>
Commit-Queue: Fabio Rocha <fabio.rocha@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#775254}
parent a9865d7b
...@@ -41,6 +41,10 @@ namespace blink { ...@@ -41,6 +41,10 @@ namespace blink {
const char NavigatorContentUtils::kSupplementName[] = "NavigatorContentUtils"; const char NavigatorContentUtils::kSupplementName[] = "NavigatorContentUtils";
namespace {
const char kToken[] = "%s";
// Changes to this list must be kept in sync with the browser-side checks in // Changes to this list must be kept in sync with the browser-side checks in
// /chrome/common/custom_handlers/protocol_handler.cc. // /chrome/common/custom_handlers/protocol_handler.cc.
static const HashSet<String>& SupportedSchemes() { static const HashSet<String>& SupportedSchemes() {
...@@ -54,50 +58,75 @@ static const HashSet<String>& SupportedSchemes() { ...@@ -54,50 +58,75 @@ static const HashSet<String>& SupportedSchemes() {
return supported_schemes; return supported_schemes;
} }
static bool VerifyCustomHandlerURL(const Document& document, bool VerifyCustomHandlerURLSyntax(const KURL& full_url,
const String& url, const KURL& base_url,
ExceptionState& exception_state) { const String& user_url,
String& error_message) {
// The specification requires that it is a SyntaxError if the "%s" token is // The specification requires that it is a SyntaxError if the "%s" token is
// not present. // not present.
static const char kToken[] = "%s"; int index = user_url.Find(kToken);
int index = url.Find(kToken);
if (-1 == index) { if (-1 == index) {
exception_state.ThrowDOMException( error_message =
DOMExceptionCode::kSyntaxError, "The url provided ('" + user_url + "') does not contain '%s'.";
"The url provided ('" + url + "') does not contain '%s'.");
return false; return false;
} }
// It is also a SyntaxError if the custom handler URL, as created by removing // It is also a SyntaxError if the custom handler URL, as created by removing
// the "%s" token and prepending the base url, does not resolve. // the "%s" token and prepending the base url, does not resolve.
String new_url = url; if (full_url.IsEmpty() || !full_url.IsValid()) {
new_url.Remove(index, base::size(kToken) - 1); error_message =
KURL kurl = document.CompleteURL(new_url);
if (kurl.IsEmpty() || !kurl.IsValid()) {
exception_state.ThrowDOMException(
DOMExceptionCode::kSyntaxError,
"The custom handler URL created by removing '%s' and prepending '" + "The custom handler URL created by removing '%s' and prepending '" +
document.BaseURL().GetString() + "' is invalid."); base_url.GetString() + "' is invalid.";
return false; return false;
} }
return true;
}
static bool VerifyCustomHandlerURLSecurity(const Document& document,
const KURL& full_url,
String& error_message) {
// Although not required by the spec, the spec allows additional security // Although not required by the spec, the spec allows additional security
// checks. Bugs have arisen from allowing non-http/https URLs, e.g. // 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 // https://crbug.com/971917 and it doesn't make a lot of sense to support
// them. We do 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")) { if (!full_url.ProtocolIsInHTTPFamily() &&
exception_state.ThrowSecurityError( !full_url.ProtocolIs("chrome-extension")) {
error_message =
"The scheme of the url provided must be 'https' or " "The scheme of the url provided must be 'https' or "
"'chrome-extension'."); "'chrome-extension'.";
return false; return false;
} }
// The specification says that the API throws SecurityError exception if the // The specification says that the API throws SecurityError exception if the
// URL's origin differs from the document's origin. // URL's origin differs from the document's origin.
if (!document.GetSecurityOrigin()->CanRequest(kurl)) { if (!document.GetSecurityOrigin()->CanRequest(full_url)) {
exception_state.ThrowSecurityError( error_message =
"Can only register custom handler in the document's origin."); "Can only register custom handler in the document's origin.";
return false;
}
return true;
}
static bool VerifyCustomHandlerURL(const Document& document,
const String& user_url,
ExceptionState& exception_state) {
String new_url = user_url;
new_url.Remove(user_url.Find(kToken), base::size(kToken) - 1);
KURL full_url = document.CompleteURL(new_url);
KURL base_url = document.BaseURL();
String error_message;
if (!VerifyCustomHandlerURLSyntax(full_url, base_url, user_url,
error_message)) {
exception_state.ThrowDOMException(DOMExceptionCode::kSyntaxError,
error_message);
return false;
}
if (!VerifyCustomHandlerURLSecurity(document, full_url, error_message)) {
exception_state.ThrowSecurityError(error_message);
return false; return false;
} }
...@@ -120,36 +149,35 @@ static bool IsValidWebSchemeName(const String& protocol) { ...@@ -120,36 +149,35 @@ static bool IsValidWebSchemeName(const String& protocol) {
return true; return true;
} }
static bool VerifyCustomHandlerScheme(const String& scheme, bool VerifyCustomHandlerScheme(const String& scheme, String& error_string) {
ExceptionState& exception_state) {
if (!IsValidProtocol(scheme)) { if (!IsValidProtocol(scheme)) {
exception_state.ThrowSecurityError( error_string = "The scheme name '" + scheme +
"The scheme name '" + scheme + "' is not allowed by URI syntax (RFC3986).";
"' is not allowed by URI syntax (RFC3986).");
return false; return false;
} }
if (scheme.StartsWithIgnoringASCIICase("web+")) { if (scheme.StartsWithIgnoringASCIICase("web+")) {
if (IsValidWebSchemeName(scheme)) if (IsValidWebSchemeName(scheme))
return true; return true;
exception_state.ThrowSecurityError( error_string =
"The scheme name '" + scheme + "The scheme name '" + scheme +
"' is not allowed. Schemes starting with 'web+' must be followed by " "' is not allowed. Schemes starting with 'web+' must be followed by "
"one or more ASCII letters."); "one or more ASCII letters.";
return false; return false;
} }
if (SupportedSchemes().Contains(scheme.LowerASCII())) if (SupportedSchemes().Contains(scheme.LowerASCII()))
return true; return true;
exception_state.ThrowSecurityError( error_string = "The scheme '" + scheme +
"The scheme '" + scheme + "' doesn't belong to the scheme allowlist. "
"' doesn't belong to the scheme allowlist. " "Please prefix non-allowlisted schemes "
"Please prefix non-allowlisted schemes " "with the string 'web+'.";
"with the string 'web+'.");
return false; return false;
} }
} // namespace
NavigatorContentUtils& NavigatorContentUtils::From(Navigator& navigator, NavigatorContentUtils& NavigatorContentUtils::From(Navigator& navigator,
LocalFrame& frame) { LocalFrame& frame) {
NavigatorContentUtils* navigator_content_utils = NavigatorContentUtils* navigator_content_utils =
...@@ -178,8 +206,11 @@ void NavigatorContentUtils::registerProtocolHandler( ...@@ -178,8 +206,11 @@ void NavigatorContentUtils::registerProtocolHandler(
// Per the HTML specification, exceptions for arguments must be surfaced in // Per the HTML specification, exceptions for arguments must be surfaced in
// the order of the arguments. // the order of the arguments.
if (!VerifyCustomHandlerScheme(scheme, exception_state)) String error_message;
if (!VerifyCustomHandlerScheme(scheme, error_message)) {
exception_state.ThrowSecurityError(error_message);
return; return;
}
if (!VerifyCustomHandlerURL(*document, url, exception_state)) if (!VerifyCustomHandlerURL(*document, url, exception_state))
return; return;
...@@ -213,8 +244,11 @@ void NavigatorContentUtils::unregisterProtocolHandler( ...@@ -213,8 +244,11 @@ void NavigatorContentUtils::unregisterProtocolHandler(
Document* document = frame->GetDocument(); Document* document = frame->GetDocument();
DCHECK(document); DCHECK(document);
if (!VerifyCustomHandlerScheme(scheme, exception_state)) String error_message;
if (!VerifyCustomHandlerScheme(scheme, error_message)) {
exception_state.ThrowSecurityError(error_message);
return; return;
}
if (!VerifyCustomHandlerURL(*document, url, exception_state)) if (!VerifyCustomHandlerURL(*document, url, exception_state))
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment