Skip to content

Commit

Permalink
Change default CSP for WebUI to disallow eval/Function.
Browse files Browse the repository at this point in the history
Only a few WebUI pages need eval() and Function(). Explicitly allow
those functions on such pages, instead of having it as the default
for all WebUI pages.

Specifically those pages are:

chrome://appcache-internals
chrome://components
chrome://conflicts
chrome://domain-reliability-internals
chrome://download-internals
crhome://dummyurl (only exists in tests)
chrome://flags
chrome://flash
chrome://gpu
chrome://indexeddb-internals
chrome://invalidations
chrome://nacl
chrome://net-internals
chrome://ntp-tiles-internals
chrome://serviceworker-internals
chrome://signin-internals
chrome://snippets-internals
chrome://supervised-user-internals
chrome://sync-internals
chrome://system

which either use third_party/jstemplate/ in prod, or use Mock4JS during testing
causing them to depend on eval/Function.

Note that this CL does not affect iOS WebUI, since it currently provides no
mechanism for overriding CSP for certain pages.

Bug: 525224
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Iaa3cbbca52a35dbd60e4d6fa42c5f324869859d9
Reviewed-on: https://chromium-review.googlesource.com/1065497
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567027}
  • Loading branch information
freshp86 authored and Commit Bot committed Jun 13, 2018
1 parent 385dab7 commit 75fa38f
Show file tree
Hide file tree
Showing 28 changed files with 53 additions and 21 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/components_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ content::WebUIDataSource* CreateComponentsUIHTMLSource(Profile* profile) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIComponentsHost);

source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->AddLocalizedString("componentsTitle", IDS_COMPONENTS_TITLE);
source->AddLocalizedString("componentsNoneInstalled",
IDS_COMPONENTS_NONE_INSTALLED);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/conflicts_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace {
content::WebUIDataSource* CreateConflictsUIHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIConflictsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->AddLocalizedString("loadingMessage", IDS_CONFLICTS_LOADING_MESSAGE);
source->AddLocalizedString("modulesLongTitle",
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/domain_reliability_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ DomainReliabilityInternalsUI::DomainReliabilityInternalsUI(
: content::WebUIController(web_ui) {
content::WebUIDataSource* html_source = content::WebUIDataSource::Create(
chrome::kChromeUIDomainReliabilityInternalsHost);
html_source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
html_source->AddResourcePath("domain_reliability_internals.css",
IDR_DOMAIN_RELIABILITY_INTERNALS_CSS);
html_source->AddResourcePath("domain_reliability_internals.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ DownloadInternalsUI::DownloadInternalsUI(content::WebUI* web_ui)
// chrome://download-internals source.
content::WebUIDataSource* html_source =
content::WebUIDataSource::Create(chrome::kChromeUIDownloadInternalsHost);
html_source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

// Required resources.
html_source->SetJsonPath("strings.js");
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/ui/webui/extensions/extensions_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ std::string GetLoadTimeClasses(bool in_dev_mode) {
content::WebUIDataSource* CreateMdExtensionsSource(bool in_dev_mode) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIExtensionsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self';");

source->SetJsonPath("strings.js");

constexpr LocalizedString localized_strings[] = {
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/flags_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ namespace {
content::WebUIDataSource* CreateFlagsUIHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIFlagsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->AddLocalizedString(flags_ui::kFlagsRestartNotice,
IDS_FLAGS_UI_RELAUNCH_NOTICE);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/flash_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ const char kFlashPlugin[] = "Flash plugin";
content::WebUIDataSource* CreateFlashUIHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIFlashHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->AddLocalizedString("loadingMessage", IDS_FLASH_LOADING_MESSAGE);
source->AddLocalizedString("flashLongTitle", IDS_FLASH_TITLE_MESSAGE);
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/webui/invalidations_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ content::WebUIDataSource* CreateInvalidationsHTMLSource() {
// This method does not fire when refreshing the page
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIInvalidationsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->AddResourcePath("about_invalidations.js", IDR_ABOUT_INVALIDATIONS_JS);
source->SetDefaultResource(IDR_ABOUT_INVALIDATIONS_HTML);
source->UseGzip();
Expand All @@ -35,4 +37,3 @@ InvalidationsUI::InvalidationsUI(content::WebUI* web_ui)
}

InvalidationsUI::~InvalidationsUI() { }

2 changes: 0 additions & 2 deletions chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ void AddLocalizedString(content::WebUIDataSource* source,
content::WebUIDataSource* CreateMdBookmarksUIHTMLSource(Profile* profile) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIBookmarksHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self';");

// Localized strings (alphabetical order).
AddLocalizedString(source, "addBookmarkTitle",
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ namespace {
content::WebUIDataSource* CreateDownloadsUIHTMLSource(Profile* profile) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIDownloadsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self';");

source->AddLocalizedString("title", IDS_DOWNLOAD_TITLE);
source->AddLocalizedString("searchResultsFor", IDS_SEARCH_RESULTS);
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/webui/md_history_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ content::WebUIDataSource* CreateMdHistoryUIHTMLSource(Profile* profile,
bool use_test_title) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUIHistoryHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self';");

// Localized strings (alphabetical order).
source->AddLocalizedString("bookmarked", IDS_HISTORY_ENTRY_BOOKMARKED);
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/webui/nacl_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ namespace {
content::WebUIDataSource* CreateNaClUIHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUINaClHost);

source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->SetJsonPath("strings.js");
source->AddResourcePath("about_nacl.css", IDR_ABOUT_NACL_CSS);
source->AddResourcePath("about_nacl.js", IDR_ABOUT_NACL_JS);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/net_internals/net_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ net::HttpNetworkSession* GetHttpNetworkSession(
content::WebUIDataSource* CreateNetInternalsHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUINetInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->SetDefaultResource(IDR_NET_INTERNALS_INDEX_HTML);
source->AddResourcePath("index.js", IDR_NET_INTERNALS_INDEX_JS);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/ntp_tiles_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ void ChromeNTPTilesInternalsMessageHandlerClient::CallJavascriptFunctionVector(
content::WebUIDataSource* CreateNTPTilesInternalsHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUINTPTilesInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->AddResourcePath("ntp_tiles_internals.js", IDR_NTP_TILES_INTERNALS_JS);
source->AddResourcePath("ntp_tiles_internals.css",
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/webui/settings/md_settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,6 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)

content::WebUIDataSource* html_source =
content::WebUIDataSource::Create(chrome::kChromeUISettingsHost);
html_source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self';");

#if defined(OS_WIN)
AddSettingsPageUIHandler(std::make_unique<ChromeCleanupHandler>(profile));
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/signin_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ namespace {
content::WebUIDataSource* CreateSignInInternalsHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUISignInInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->SetJsonPath("strings.js");
source->AddResourcePath("signin_internals.js", IDR_SIGNIN_INTERNALS_INDEX_JS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ SnippetsInternalsUI::SnippetsInternalsUI(content::WebUI* web_ui)
: ui::MojoWebUIController(web_ui), binding_(this) {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUISnippetsInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->AddResourcePath("snippets_internals.css", IDR_SNIPPETS_INTERNALS_CSS);
source->AddResourcePath("snippets_internals.js", IDR_SNIPPETS_INTERNALS_JS);
source->AddResourcePath("snippets_internals.mojom.js",
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/supervised_user_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ namespace {
content::WebUIDataSource* CreateSupervisedUserInternalsHTMLSource() {
content::WebUIDataSource* source = content::WebUIDataSource::Create(
chrome::kChromeUISupervisedUserInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->AddResourcePath("supervised_user_internals.js",
IDR_SUPERVISED_USER_INTERNALS_JS);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/sync_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace {
content::WebUIDataSource* CreateSyncInternalsHTMLSource() {
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUISyncInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->SetJsonPath("strings.js");
source->AddResourcePath(syncer::sync_ui_util::kSyncIndexJS,
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/webui/system_info_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SystemInfoUIHTMLSource : public content::URLDataSource{
return "text/html";
}
std::string GetContentSecurityPolicyScriptSrc() const override {
// 'unsafe-inline' is added to script-src.
// 'unsafe-eval' and 'unsafe-inline' are added to script-src.
return "script-src 'self' chrome://resources 'unsafe-eval' "
"'unsafe-inline';";
}
Expand Down
7 changes: 7 additions & 0 deletions chrome/test/base/web_ui_browser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,13 @@ class MockWebUIDataSource : public content::URLDataSource {
return "text/html";
}

// Append 'unsave-eval' to the default script-src CSP policy, since it is
// needed by some tests using chrome://dummyurl (because they depend on
// Mock4JS, see crbug.com/844820).
std::string GetContentSecurityPolicyScriptSrc() const override {
return "script-src chrome://resources 'self' 'unsafe-eval';";
}

DISALLOW_COPY_AND_ASSIGN(MockWebUIDataSource);
};

Expand Down
5 changes: 4 additions & 1 deletion chrome/test/data/webui/accessibility_audit_browsertest.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function WebUIAccessibilityAuditBrowserTest() {}
WebUIAccessibilityAuditBrowserTest.prototype = {
__proto__: testing.Test.prototype,

browsePreload: 'chrome://terms',
browsePreload: 'chrome://dummyurl',

runAccessibilityChecks: true,
accessibilityIssuesAreErrors: true,
Expand Down Expand Up @@ -91,6 +91,9 @@ function addAuditFailures() {
style.innerText = 'body { background-color: #ffff }\n' +
'p { color: #ffffff }';
document.head.appendChild(style);
var p = document.createElement('p');
p.textContent = 'Low contrast ratio text';
document.body.appendChild(p);

// Bad ARIA role
var div = document.createElement('div');
Expand Down
2 changes: 2 additions & 0 deletions content/browser/appcache/appcache_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ AppCacheInternalsUI::AppCacheInternalsUI(WebUI* web_ui)

WebUIDataSource* source =
WebUIDataSource::Create(kChromeUIAppCacheInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->SetJsonPath("strings.js");
source->AddResourcePath("appcache_internals.js", IDR_APPCACHE_INTERNALS_JS);
Expand Down
2 changes: 2 additions & 0 deletions content/browser/gpu/gpu_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ namespace {

WebUIDataSource* CreateGpuHTMLSource() {
WebUIDataSource* source = WebUIDataSource::Create(kChromeUIGpuHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");

source->SetJsonPath("strings.js");
source->AddResourcePath("gpu_internals.js", IDR_GPU_INTERNALS_JS);
Expand Down
2 changes: 2 additions & 0 deletions content/browser/indexed_db/indexed_db_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ IndexedDBInternalsUI::IndexedDBInternalsUI(WebUI* web_ui)

WebUIDataSource* source =
WebUIDataSource::Create(kChromeUIIndexedDBInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->SetJsonPath("strings.js");
source->AddResourcePath("indexeddb_internals.js",
IDR_INDEXED_DB_INTERNALS_JS);
Expand Down
2 changes: 2 additions & 0 deletions content/browser/service_worker/service_worker_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ ServiceWorkerInternalsUI::ServiceWorkerInternalsUI(WebUI* web_ui)
: WebUIController(web_ui), next_partition_id_(0) {
WebUIDataSource* source =
WebUIDataSource::Create(kChromeUIServiceWorkerInternalsHost);
source->OverrideContentSecurityPolicyScriptSrc(
"script-src chrome://resources 'self' 'unsafe-eval';");
source->SetJsonPath("strings.js");
source->AddResourcePath("serviceworker_internals.js",
IDR_SERVICE_WORKER_INTERNALS_JS);
Expand Down
7 changes: 3 additions & 4 deletions content/public/browser/url_data_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ bool URLDataSource::ShouldAddContentSecurityPolicy() const {
}

std::string URLDataSource::GetContentSecurityPolicyScriptSrc() const {
// Specific resources require unsafe-eval in the Content Security Policy.
// TODO(tsepez,mfoltz): Remove 'unsafe-eval' when tests have been fixed to
// not use eval()/new Function(). http://crbug.com/525224
return "script-src chrome://resources 'self' 'unsafe-eval';";
// Note: Do not add 'unsafe-eval' here. Instead override CSP for the
// specific pages that need it, see context http://crbug.com/525224.
return "script-src chrome://resources 'self';";
}

std::string URLDataSource::GetContentSecurityPolicyObjectSrc() const {
Expand Down
4 changes: 2 additions & 2 deletions content/public/browser/url_data_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ class CONTENT_EXPORT URLDataSource {
// may be marginally better than disabling CSP outright.
// Do not override this method without first contacting the chrome security
// team.
// By default, "script-src chrome://resources 'self' 'unsafe-eval';" is added
// to CSP. Override to change this.
// By default, "script-src chrome://resources 'self';" is added to CSP.
// Override to change this.
virtual std::string GetContentSecurityPolicyScriptSrc() const;

// It is OK to override the following methods to a custom CSP directive
Expand Down

0 comments on commit 75fa38f

Please sign in to comment.