Never bypass CSP in WebUI pages
BUG=668645
TEST=browser_test --gtest_filter=ExtensionCSPBypassTest.LoadWebAccessibleScript
Change-Id: I03a96a742b06ca6ec81fad95aae17cbdb11e1cce
Reviewed-on: https://chromium-review.googlesource.com/567499
Commit-Queue: Rob Wu <rob@robwu.nl>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528343}diff --git a/chrome/browser/extensions/extension_csp_bypass_browsertest.cc b/chrome/browser/extensions/extension_csp_bypass_browsertest.cc
new file mode 100644
index 0000000..bd852ab
--- /dev/null
+++ b/chrome/browser/extensions/extension_csp_bypass_browsertest.cc
@@ -0,0 +1,119 @@
+// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "extensions/common/value_builder.h"
+
+namespace extensions {
+
+namespace {
+
+class ExtensionCSPBypassTest : public ExtensionBrowserTest {
+ public:
+ ExtensionCSPBypassTest() {}
+
+ void SetUpOnMainThread() override {
+ ExtensionBrowserTest::SetUpOnMainThread();
+ ASSERT_TRUE(embedded_test_server()->Start());
+ }
+
+ protected:
+ const Extension* AddExtension(bool is_component, bool all_urls_permission) {
+ auto dir = std::make_unique<TestExtensionDir>();
+
+ std::string unique_name = base::StringPrintf(
+ "component=%d, all_urls=%d", is_component, all_urls_permission);
+ DictionaryBuilder manifest;
+ manifest.Set("name", unique_name)
+ .Set("version", "1")
+ .Set("manifest_version", 2)
+ .Set("web_accessible_resources", ListBuilder().Append("*").Build());
+
+ if (all_urls_permission) {
+ manifest.Set("permissions", ListBuilder().Append("<all_urls>").Build());
+ }
+ if (is_component) {
+ // LoadExtensionAsComponent requires the manifest to contain a key.
+ std::string key;
+ EXPECT_TRUE(Extension::ProducePEM(unique_name, &key));
+ manifest.Set("key", key);
+ }
+
+ dir->WriteFile(FILE_PATH_LITERAL("script.js"), "");
+ dir->WriteManifest(manifest.ToJSON());
+
+ const Extension* extension = nullptr;
+ if (is_component) {
+ extension = LoadExtensionAsComponent(dir->UnpackedPath());
+ } else {
+ extension = LoadExtension(dir->UnpackedPath());
+ }
+ CHECK(extension);
+ temp_dirs_.push_back(std::move(dir));
+ return extension;
+ }
+
+ bool CanLoadScript(const Extension* extension) {
+ content::RenderFrameHost* rfh =
+ browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+ std::string code = base::StringPrintf(
+ R"(
+ var s = document.createElement('script');
+ s.src = '%s';
+ s.onload = function() {
+ // Not blocked by CSP.
+ window.domAutomationController.send(true);
+ };
+ s.onerror = function() {
+ // Blocked by CSP.
+ window.domAutomationController.send(false);
+ };
+ document.body.appendChild(s);)",
+ extension->GetResourceURL("script.js").spec().c_str());
+ bool script_loaded = false;
+ EXPECT_TRUE(ExecuteScriptAndExtractBool(rfh, code, &script_loaded));
+ return script_loaded;
+ }
+
+ private:
+ std::vector<std::unique_ptr<TestExtensionDir>> temp_dirs_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExtensionCSPBypassTest);
+};
+
+} // namespace
+
+IN_PROC_BROWSER_TEST_F(ExtensionCSPBypassTest, LoadWebAccessibleScript) {
+ const Extension* component_ext_with_permission = AddExtension(true, true);
+ const Extension* component_ext_without_permission = AddExtension(true, false);
+ const Extension* ext_with_permission = AddExtension(false, true);
+ const Extension* ext_without_permission = AddExtension(false, false);
+
+ // chrome-extension:-URLs can always bypass CSP in normal pages.
+ GURL non_webui_url(embedded_test_server()->GetURL("/empty.html"));
+ ui_test_utils::NavigateToURL(browser(), non_webui_url);
+
+ EXPECT_TRUE(CanLoadScript(component_ext_with_permission));
+ EXPECT_TRUE(CanLoadScript(component_ext_without_permission));
+ EXPECT_TRUE(CanLoadScript(ext_with_permission));
+ EXPECT_TRUE(CanLoadScript(ext_without_permission));
+
+ // chrome-extension:-URLs can never bypass CSP in WebUI.
+ ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIVersionURL));
+
+ EXPECT_FALSE(CanLoadScript(component_ext_with_permission));
+ EXPECT_FALSE(CanLoadScript(component_ext_without_permission));
+ EXPECT_FALSE(CanLoadScript(ext_with_permission));
+ EXPECT_FALSE(CanLoadScript(ext_without_permission));
+}
+
+} // namespace extensions
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 1802bfe..44d59b6 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -1216,6 +1216,7 @@
"../browser/extensions/extension_browsertest.cc",
"../browser/extensions/extension_browsertest.h",
"../browser/extensions/extension_context_menu_browsertest.cc",
+ "../browser/extensions/extension_csp_bypass_browsertest.cc",
"../browser/extensions/extension_disabled_ui_browsertest.cc",
"../browser/extensions/extension_dom_clipboard_apitest.cc",
"../browser/extensions/extension_fileapi_apitest.cc",
diff --git a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
index bb3994c..fbb0abb 100644
--- a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
@@ -684,9 +684,13 @@
// regardless of parser state. Once we have more data via the
// 'ScriptWithCSPBypassingScheme*' metrics, make a decision about what
// behavior to ship. https://crbug.com/653521
- if (parser_disposition == kNotParserInserted ||
- !RuntimeEnabledFeatures::
- ExperimentalContentSecurityPolicyFeaturesEnabled()) {
+ if ((parser_disposition == kNotParserInserted ||
+ !RuntimeEnabledFeatures::
+ ExperimentalContentSecurityPolicyFeaturesEnabled()) &&
+ // The schemes where javascript:-URLs are blocked are usually privileged
+ // pages, so do not allow the CSP to be bypassed either.
+ !SchemeRegistry::ShouldTreatURLSchemeAsNotAllowingJavascriptURLs(
+ execution_context_->GetSecurityOrigin()->Protocol())) {
return true;
}
}
diff --git a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp
index 72f40a320..e0c3926 100644
--- a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicyTest.cpp
@@ -1176,6 +1176,52 @@
"https");
}
+TEST_F(ContentSecurityPolicyTest, CSPBypassDisabledWhenSchemeIsPrivileged) {
+ const KURL base;
+ execution_context = CreateExecutionContext();
+ execution_context->SetSecurityOrigin(secure_origin);
+ execution_context->SetURL(BlankURL());
+ csp->BindToExecutionContext(execution_context.Get());
+ csp->DidReceiveHeader("script-src http://example.com",
+ kContentSecurityPolicyHeaderTypeEnforce,
+ kContentSecurityPolicyHeaderSourceHTTP);
+
+ const KURL allowed_url("http://example.com/script.js");
+ const KURL http_url("http://not-example.com/script.js");
+ const KURL blob_url(base, "blob:http://not-example.com/uuid");
+ const KURL filesystem_url(base, "filesystem:http://not-example.com/file.js");
+
+ // The {Requests,Blob,Filesystem}AllowedWhenBypassingCSP tests have already
+ // ensured that RegisterURLSchemeAsBypassingContentSecurityPolicy works as
+ // expected.
+ //
+ // "http" is registered as bypassing CSP, but the context's scheme ("https")
+ // is marked as a privileged scheme, so the bypass rule should be ignored.
+ SchemeRegistry::RegisterURLSchemeAsBypassingContentSecurityPolicy("http");
+ SchemeRegistry::RegisterURLSchemeAsNotAllowingJavascriptURLs("https");
+
+ EXPECT_TRUE(csp->AllowScriptFromSource(
+ allowed_url, String(), IntegrityMetadataSet(), kNotParserInserted,
+ ResourceRequest::RedirectStatus::kNoRedirect,
+ SecurityViolationReportingPolicy::kSuppressReporting));
+ EXPECT_FALSE(csp->AllowScriptFromSource(
+ http_url, String(), IntegrityMetadataSet(), kNotParserInserted,
+ ResourceRequest::RedirectStatus::kNoRedirect,
+ SecurityViolationReportingPolicy::kSuppressReporting));
+ EXPECT_FALSE(csp->AllowScriptFromSource(
+ blob_url, String(), IntegrityMetadataSet(), kNotParserInserted,
+ ResourceRequest::RedirectStatus::kNoRedirect,
+ SecurityViolationReportingPolicy::kSuppressReporting));
+ EXPECT_FALSE(csp->AllowScriptFromSource(
+ filesystem_url, String(), IntegrityMetadataSet(), kNotParserInserted,
+ ResourceRequest::RedirectStatus::kNoRedirect,
+ SecurityViolationReportingPolicy::kSuppressReporting));
+
+ SchemeRegistry::RemoveURLSchemeRegisteredAsBypassingContentSecurityPolicy(
+ "http");
+ SchemeRegistry::RemoveURLSchemeAsNotAllowingJavascriptURLs("https");
+}
+
TEST_F(ContentSecurityPolicyTest, IsValidCSPAttrTest) {
// Empty string is invalid
EXPECT_FALSE(ContentSecurityPolicy::IsValidCSPAttr(""));
diff --git a/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp b/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
index e73e1b00..4b18af9 100644
--- a/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
+++ b/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
@@ -209,6 +209,12 @@
scheme);
}
+void SchemeRegistry::RemoveURLSchemeAsNotAllowingJavascriptURLs(
+ const String& scheme) {
+ GetMutableURLSchemesRegistry().not_allowing_javascript_urls_schemes.erase(
+ scheme);
+}
+
bool SchemeRegistry::ShouldTreatURLSchemeAsNotAllowingJavascriptURLs(
const String& scheme) {
DCHECK_EQ(scheme, scheme.LowerASCII());
diff --git a/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h b/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h
index d3d54a4..d77dc0c 100644
--- a/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h
+++ b/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h
@@ -83,6 +83,7 @@
// bookmarklets from running on sensitive pages).
static void RegisterURLSchemeAsNotAllowingJavascriptURLs(
const String& scheme);
+ static void RemoveURLSchemeAsNotAllowingJavascriptURLs(const String& scheme);
static bool ShouldTreatURLSchemeAsNotAllowingJavascriptURLs(
const String& scheme);
