Commit ea8b8091 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Handle invalid paths in webview accessible resources

An extension may specify an invalid path in webview accessible resources
(such as a non-relative path or an URLPattern). This would lead to the
URLPattern parsing failing. However, WebviewInfo currently uses a
constructor that always expects this parsing to succeed.

Fix this by handling invalid inputs. Unlike other parsing failures, we
add a warning (rather than throwing a hard error) in this case. This is
because existing apps with non-trivial user counts use this.

Bug: 856948
Change-Id: Ia2087dcf079250d12a1bd6211d6424a03a8a24bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450952Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815791}
parent f42eec98
......@@ -82,4 +82,18 @@ TEST_F(WebviewAccessibleResourcesManifestTest, InvalidManifest) {
"webview_accessible_resources_invalid8.json",
ErrorUtils::FormatErrorMessage(errors::kInvalidWebviewAccessibleResource,
base::NumberToString(0)));
{
// Specifying non-relative paths as accessible resources should fail. We
// raise a warning rather than a hard-error because existing apps do this
// and we don't want to break them for all existing users.
// https://crbug.com/856948.
scoped_refptr<const Extension> extension = LoadAndExpectWarning(
"webview_accessible_resources_non_relative_path.json",
ErrorUtils::FormatErrorMessage(
errors::kInvalidWebviewAccessibleResource,
base::NumberToString(0)));
EXPECT_FALSE(WebviewInfo::IsResourceWebviewAccessible(
extension.get(), "nonrelative", "a.html"));
}
}
{
"name": "test",
"version": "2",
"manifest_version": 2,
"permissions": [
"webview"
],
"app": {
"background": {
"scripts": ["test.js"]
}
},
"webview": {
"partitions": [
{
"name": "nonrelative",
"accessible_resources": ["http://*/*"]
}
]
}
}
......@@ -176,11 +176,37 @@ bool WebviewHandler::Parse(Extension* extension, base::string16* error) {
errors::kInvalidWebviewAccessibleResource, base::NumberToString(i));
return false;
}
partition_item->AddPattern(
URLPattern(URLPattern::SCHEME_EXTENSION,
Extension::GetResourceURL(extension->url(),
url_list_view[i].GetString())
.spec()));
GURL pattern_url = Extension::GetResourceURL(
extension->url(), url_list_view[i].GetString());
// If passed a non-relative URL (like http://example.com),
// Extension::GetResourceURL() will return that URL directly. (See
// https://crbug.com/1135236). Check if this happened by comparing the
// host.
if (pattern_url.host_piece() != extension->id()) {
// NOTE: Warning instead of error because there are existing apps that
// have this bug, and we don't want to hard-error on them.
// https://crbug.com/856948.
std::string warning = ErrorUtils::FormatErrorMessage(
errors::kInvalidWebviewAccessibleResource, base::NumberToString(i));
extension->AddInstallWarning(
InstallWarning(std::move(warning), keys::kWebview));
continue;
}
URLPattern pattern(URLPattern::SCHEME_EXTENSION);
if (pattern.Parse(pattern_url.spec()) !=
URLPattern::ParseResult::kSuccess) {
// NOTE: Warning instead of error because there are existing apps that
// have this bug, and we don't want to hard-error on them.
// https://crbug.com/856948.
std::string warning = ErrorUtils::FormatErrorMessage(
errors::kInvalidWebviewAccessibleResource, base::NumberToString(i));
extension->AddInstallWarning(
InstallWarning(std::move(warning), keys::kWebview));
continue;
}
partition_item->AddPattern(std::move(pattern));
}
info->AddPartitionItem(std::move(partition_item));
}
......
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