[CSP] Check inline script CSP in prepare-a-script
This CL moves the inline script CSP check from
PendingScript::ExecuteScriptBlock() (#execute-the-script-block)
to ScriptLoader::PrepareScript() (#prepare-a-script)
as spec'ed.
This CL removes Script::InlineSourceTextForCSP()
which is no longer used.
Behavior changes (the new behavior is spec-conformant and thus
this CL adds WPT tests):
- Previously <script>'s error events were fired
when inline script CSP check fails,
while after this CL the events are no longer fired.
Test: scripthash-basic-blocked-error-event.html
(Moved from layout test with expectation changes)
This CL makes Chromium's behavior align with Firefox and Safari.
- If the nonce attribute is changed or the CSP list is updated
after prepare-a-script before evaluation,
previously the new nonce/CSP were used for CSP,
while after this CL the old nonce/CSP
(at the time of prepare-a-script) is used.
Test: scriptnonce-changed-*.html
This CL makes Chromium's behavior align with Firefox.
(Safari's behavior is different from any other browsers)
This CL also adds scripthash-changed-*.html (just for symmetry
with scriptnonce-changed.html), which pass only on Chromium.
Bug: 964537
Change-Id: I8673956101d9d13708c452db23258f125cb3d256
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618262
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683391}
diff --git a/third_party/blink/renderer/core/script/classic_script.h b/third_party/blink/renderer/core/script/classic_script.h
index e64a211..2142a27 100644
--- a/third_party/blink/renderer/core/script/classic_script.h
+++ b/third_party/blink/renderer/core/script/classic_script.h
@@ -36,9 +36,6 @@
}
void RunScript(LocalFrame*, const SecurityOrigin*) override;
void RunScriptOnWorker(WorkerGlobalScope&) override;
- String InlineSourceTextForCSP() const override {
- return script_source_code_.Source().ToString();
- }
const ScriptSourceCode script_source_code_;
const SanitizeScriptErrors sanitize_script_errors_;
diff --git a/third_party/blink/renderer/core/script/js_module_script.cc b/third_party/blink/renderer/core/script/js_module_script.cc
index 8b12c6ac..694f6fe 100644
--- a/third_party/blink/renderer/core/script/js_module_script.cc
+++ b/third_party/blink/renderer/core/script/js_module_script.cc
@@ -57,8 +57,8 @@
// immediately turn the script into errored state. Thus the members will not
// be used for the speced algorithms, but may be used from inspector.
JSModuleScript* script =
- CreateInternal(source_text, modulator, result, source_url, base_url,
- options, start_position, produce_cache_data);
+ CreateInternal(source_text.length(), modulator, result, source_url,
+ base_url, options, start_position, produce_cache_data);
// <spec step="8">If result is a list of errors, then:</spec>
if (exception_state.HadException()) {
@@ -108,17 +108,15 @@
ModuleRecord record,
const KURL& base_url,
const ScriptFetchOptions& options) {
- ParkableString dummy_source_text(String("").ReleaseImpl());
KURL dummy_source_url;
- return CreateInternal(dummy_source_text, modulator, record, dummy_source_url,
- base_url, options, TextPosition::MinimumPosition(),
- nullptr);
+ return CreateInternal(0, modulator, record, dummy_source_url, base_url,
+ options, TextPosition::MinimumPosition(), nullptr);
}
// <specdef
// href="https://html.spec.whatwg.org/C/#creating-a-javascript-module-script">
JSModuleScript* JSModuleScript::CreateInternal(
- const ParkableString& source_text,
+ size_t source_text_length,
Modulator* modulator,
ModuleRecord result,
const KURL& source_url,
@@ -134,10 +132,8 @@
// <spec step="4">Set script's base URL to baseURL.</spec>
//
// <spec step="5">Set script's fetch options to options.</spec>
- //
- // [nospec] |source_text| is saved for CSP checks.
JSModuleScript* module_script = MakeGarbageCollected<JSModuleScript>(
- modulator, result, source_url, base_url, options, source_text,
+ modulator, result, source_url, base_url, options, source_text_length,
start_position, produce_cache_data);
// Step 7, a part of ParseModule(): Passing script as the last parameter
@@ -152,7 +148,7 @@
const KURL& source_url,
const KURL& base_url,
const ScriptFetchOptions& fetch_options,
- const ParkableString& source_text,
+ size_t source_text_length,
const TextPosition& start_position,
ModuleRecordProduceCacheData* produce_cache_data)
: ModuleScript(settings_object,
@@ -160,14 +156,10 @@
source_url,
base_url,
fetch_options),
- source_text_(source_text),
+ source_text_length_(source_text_length),
start_position_(start_position),
produce_cache_data_(produce_cache_data) {}
-String JSModuleScript::InlineSourceTextForCSP() const {
- return source_text_.ToString();
-}
-
void JSModuleScript::ProduceCache() {
if (!produce_cache_data_)
return;
@@ -176,7 +168,7 @@
v8::Isolate* isolate = script_state->GetIsolate();
ScriptState::Scope scope(script_state);
- V8CodeCache::ProduceCache(isolate, produce_cache_data_, source_text_.length(),
+ V8CodeCache::ProduceCache(isolate, produce_cache_data_, source_text_length_,
SourceURL(), StartPosition());
produce_cache_data_ = nullptr;
diff --git a/third_party/blink/renderer/core/script/js_module_script.h b/third_party/blink/renderer/core/script/js_module_script.h
index 61089f8..5d188c8b 100644
--- a/third_party/blink/renderer/core/script/js_module_script.h
+++ b/third_party/blink/renderer/core/script/js_module_script.h
@@ -50,7 +50,7 @@
const KURL& source_url,
const KURL& base_url,
const ScriptFetchOptions&,
- const ParkableString& source_text,
+ size_t source_text_length,
const TextPosition& start_position,
ModuleRecordProduceCacheData*);
~JSModuleScript() override = default;
@@ -64,7 +64,7 @@
friend class ModuleScriptTest;
static JSModuleScript* CreateInternal(
- const ParkableString& source_text,
+ size_t source_text_length,
Modulator*,
ModuleRecord,
const KURL& source_url,
@@ -74,10 +74,9 @@
ModuleRecordProduceCacheData* produce_cache_data);
const TextPosition& StartPosition() const { return start_position_; }
- String InlineSourceTextForCSP() const override;
- // For CSP check.
- const ParkableString source_text_;
+ // For V8CodeCache statistics.
+ const size_t source_text_length_;
const TextPosition start_position_;
diff --git a/third_party/blink/renderer/core/script/pending_script.cc b/third_party/blink/renderer/core/script/pending_script.cc
index b8fe2d8a..5fefbfc 100644
--- a/third_party/blink/renderer/core/script/pending_script.cc
+++ b/third_party/blink/renderer/core/script/pending_script.cc
@@ -29,7 +29,6 @@
#include "third_party/blink/renderer/bindings/core/v8/script_controller.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/document_parser_timing.h"
-#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/script/ignore_destructive_write_count_incrementer.h"
#include "third_party/blink/renderer/core/script/script_element_base.h"
@@ -155,20 +154,6 @@
Script* script = GetSource(document_url);
- if (script && !IsExternal()) {
- AtomicString nonce = element_->GetNonceForElement();
- if (!element_->AllowInlineScriptForCSP(nonce, StartingPosition().line_,
- script->InlineSourceTextForCSP())) {
- // Consider as if:
- //
- // <spec step="2">If the script's script is null, ...</spec>
- //
- // retrospectively, if the CSP check fails, which is considered as load
- // failure.
- script = nullptr;
- }
- }
-
const bool was_canceled = WasCanceled();
const bool is_external = IsExternal();
const bool created_during_document_write = WasCreatedDuringDocumentWrite();
diff --git a/third_party/blink/renderer/core/script/script.h b/third_party/blink/renderer/core/script/script.h
index d7951dc1..2226029 100644
--- a/third_party/blink/renderer/core/script/script.h
+++ b/third_party/blink/renderer/core/script/script.h
@@ -35,9 +35,6 @@
virtual void RunScript(LocalFrame*, const SecurityOrigin*) = 0;
virtual void RunScriptOnWorker(WorkerGlobalScope&) = 0;
- // For CSP check for inline scripts.
- virtual String InlineSourceTextForCSP() const = 0;
-
const ScriptFetchOptions& FetchOptions() const { return fetch_options_; }
const KURL& BaseURL() const { return base_url_; }
diff --git a/third_party/blink/renderer/core/script/script_loader.cc b/third_party/blink/renderer/core/script/script_loader.cc
index 4666d85..98dc905 100644
--- a/third_party/blink/renderer/core/script/script_loader.cc
+++ b/third_party/blink/renderer/core/script/script_loader.cc
@@ -415,7 +415,19 @@
TextPosition position =
is_in_document_write ? TextPosition() : script_start_position;
- // 13.
+ // <spec step="13">If the script element does not have a src content
+ // attribute, and the Should element's inline behavior be blocked by Content
+ // Security Policy? algorithm returns "Blocked" when executed upon the script
+ // element, "script", and source text, then return. The script is not
+ // executed. [CSP]</spec>
+ if (!element_->HasSourceAttribute() &&
+ !element_->AllowInlineScriptForCSP(element_->GetNonceForElement(),
+ position.line_,
+ element_->TextFromChildren())) {
+ return false;
+ }
+
+ // 14.
if (!IsScriptForEventSupported())
return false;
diff --git a/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc b/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc
index 7e04194..24bf5bb2 100644
--- a/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc
+++ b/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc
@@ -148,17 +148,6 @@
return v8::Undefined(reinterpret_cast<v8::Isolate*>(isolate));
}
-String ValueWrapperSyntheticModuleScript::InlineSourceTextForCSP() const {
- // We don't construct a ValueWrapperSyntheticModuleScript with the original
- // source, but instead construct it from the originally parsed
- // text. If a need arises for the original module source to be used later,
- // ValueWrapperSyntheticModuleScript will need to be modified such that its
- // constructor takes this source text as an additional parameter and stashes
- // it on the ValueWrapperSyntheticModuleScript.
- NOTREACHED();
- return "";
-}
-
void ValueWrapperSyntheticModuleScript::Trace(Visitor* visitor) {
visitor->Trace(export_value_);
ModuleScript::Trace(visitor);
diff --git a/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.h b/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.h
index ccc7716..1200e84e 100644
--- a/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.h
+++ b/third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.h
@@ -65,7 +65,6 @@
v8::Local<v8::Context> context,
v8::Local<v8::Module> module);
- String InlineSourceTextForCSP() const override;
void Trace(blink::Visitor* visitor) override;
private:
diff --git a/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/1.1/scripthash-basic-blocked-error-event.html b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-basic-blocked-error-event.html
similarity index 81%
rename from third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/1.1/scripthash-basic-blocked-error-event.html
rename to third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-basic-blocked-error-event.html
index d0edacc1..62b8693 100644
--- a/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/1.1/scripthash-basic-blocked-error-event.html
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-basic-blocked-error-event.html
@@ -6,5 +6,5 @@
<meta http-equiv="Content-Security-Policy" content="script-src 'self' 'sha256-deadbeef'"></meta>
</head>
<body>
- <script src="../resources/fail-to-inject-script.js"></script>
+ <script src="support/inline-script-should-be-blocked.js"></script>
</body>
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-1.html b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-1.html
new file mode 100644
index 0000000..9da41dd
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-1.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<head>
+ <title>CSP inline script check is done at #prepare-a-script (hash)</title>
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <!--
+ 'log1 += 'scr1 at #prepare-a-script';' => 'sha256-sI+xsvqqUw0LQQGgsgkYoXKWhlGgaCqsqVbPx0Z2A4s=' (allowed)
+ 'log1 += 'scr1 at #execute-the-script-block';' => 'sha256-Vtap5AhPN9kbQAbWqObJexCvNDexqoIwo4XsABQBqcg=' (blocked)
+ -->
+ <meta http-equiv="Content-Security-Policy" content="script-src 'self' 'nonce-abc' 'sha256-sI+xsvqqUw0LQQGgsgkYoXKWhlGgaCqsqVbPx0Z2A4s='"></meta>
+</head>
+<!--
+ "Should element's inline behavior be blocked by Content Security Policy?"
+ is executed at the time of https://html.spec.whatwg.org/C/#prepare-a-script,
+ not at https://html.spec.whatwg.org/C/#execute-the-script-block.
+ So when innerText is modified after #prepare-a-script, the text BEFORE
+ the modification is used for hash check.
+-->
+<script nonce="abc">
+let log1 = '';
+</script>
+
+<!-- Execution order:
+ async script is executed
+ -> stylesheet is loaded
+ -> inline script is executed. -->
+<link rel="stylesheet" href="support/empty.css?dummy=1&pipe=trickle(d2)" type="text/css">
+<script src="support/change-scripthash-before-execute.js?dummy=1&pipe=trickle(d1)" async></script>
+<script id="scr1">log1 += 'scr1 at #prepare-a-script';</script>
+
+<script nonce="abc">
+test(() => {
+ assert_equals(log1, 'scr1 at #prepare-a-script');
+}, 'scr1.innerText before modification should not be blocked');
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-2.html b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-2.html
new file mode 100644
index 0000000..927d60a8
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-2.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<head>
+ <title>CSP inline script check is done at #prepare-a-script (hash)</title>
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <!--
+ 'log2 += 'scr2 at #prepare-a-script';' => 'sha256-9vE5NuHfEDoLvk3nPZPDX2/mnG+ZwKhpPuwQZwCDGc4=' (blocked)
+ 'log2 += 'scr2 at #execute-the-script-block';' => 'sha256-3AdhWTFuyxSUPxmqpTJaFRx3R5WNcyGw57lFoj1rTXw=' (allowed)
+ -->
+ <meta http-equiv="Content-Security-Policy" content="script-src 'self' 'nonce-abc' 'sha256-3AdhWTFuyxSUPxmqpTJaFRx3R5WNcyGw57lFoj1rTXw='"></meta>
+</head>
+<!--
+ "Should element's inline behavior be blocked by Content Security Policy?"
+ is executed at the time of https://html.spec.whatwg.org/C/#prepare-a-script,
+ not at https://html.spec.whatwg.org/C/#execute-the-script-block.
+ So when innerText is modified after #prepare-a-script, the text BEFORE
+ the modification is used for hash check.
+-->
+<script nonce="abc">
+let log2 = '';
+</script>
+
+<!-- Execution order:
+ async script is executed
+ -> stylesheet is loaded
+ -> inline script is executed. -->
+<link rel="stylesheet" href="support/empty.css?dummy=2&pipe=trickle(d2)" type="text/css">
+<script src="support/change-scripthash-before-execute.js?dummy=2&pipe=trickle(d1)" async></script>
+<script id="scr2">log2 += 'scr2 at #prepare-a-script';</script>
+
+<script nonce="abc">
+test(() => {
+ assert_equals(log2, '');
+}, 'scr2.innerText before modification should be blocked');
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-1.html b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-1.html
new file mode 100644
index 0000000..75f92f35
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-1.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<head>
+ <title>CSP inline script check is done at #prepare-a-script (nonce)</title>
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <meta http-equiv="Content-Security-Policy" content="script-src 'self' 'nonce-abc' 'sha256-deadbeef'"></meta>
+</head>
+<!--
+ "Should element's inline behavior be blocked by Content Security Policy?"
+ is executed at the time of https://html.spec.whatwg.org/C/#prepare-a-script,
+ not at https://html.spec.whatwg.org/C/#execute-the-script-block.
+ So when nonce is modified after #prepare-a-script, the nonce BEFORE
+ the modification is used for hash check.
+-->
+<script nonce="abc">
+let log1 = '';
+</script>
+
+<!-- Execution order:
+ async script is executed
+ -> stylesheet is loaded
+ -> inline script is executed. -->
+<link rel="stylesheet" href="support/empty.css?dummy=3&pipe=trickle(d2)" type="text/css">
+<script src="support/change-scriptnonce-before-execute.js?dummy=3&pipe=trickle(d1)" async></script>
+<script id="scr1" nonce="abc">log1 += 'scr1 executed';</script>
+
+<script nonce="abc">
+test(() => {
+ assert_equals(log1, 'scr1 executed');
+}, 'scr1 nonce before modification should not be blocked');
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-2.html b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-2.html
new file mode 100644
index 0000000..f2321dd
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-2.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<head>
+ <title>CSP inline script check is done at #prepare-a-script (nonce)</title>
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <meta http-equiv="Content-Security-Policy" content="script-src 'self' 'nonce-abc' 'sha256-deadbeef'"></meta>
+</head>
+<!--
+ "Should element's inline behavior be blocked by Content Security Policy?"
+ is executed at the time of https://html.spec.whatwg.org/C/#prepare-a-script,
+ not at https://html.spec.whatwg.org/C/#execute-the-script-block.
+ So when nonce is modified after #prepare-a-script, the nonce BEFORE
+ the modification is used for hash check.
+-->
+<script nonce="abc">
+let log2 = '';
+</script>
+
+<!-- Execution order:
+ async script is executed
+ -> stylesheet is loaded
+ -> inline script is executed. -->
+<link rel="stylesheet" href="support/empty.css?dummy=4&pipe=trickle(d2)" type="text/css">
+<script src="support/change-scriptnonce-before-execute.js?dummy=4&pipe=trickle(d1)" async></script>
+<script id="scr2" nonce="wrong">log2 += 'scr2 executed';</script>
+
+<script nonce="abc">
+test(() => {
+ assert_equals(log2, '');
+}, 'scr2 nonce before modification should be blocked');
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scripthash-before-execute.js b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scripthash-before-execute.js
new file mode 100644
index 0000000..a04e857
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scripthash-before-execute.js
@@ -0,0 +1,10 @@
+// This script is executed after |scr1| and |scr2| are inserted into DOM
+// before their execution (if not blocked by CSP).
+if (document.getElementById("scr1")) {
+ document.getElementById("scr1").innerText =
+ "log1 += 'scr1 at #execute-the-script-block';";
+}
+if (document.getElementById("scr2")) {
+ document.getElementById("scr2").innerText =
+ "log2 += 'scr2 at #execute-the-script-block';";
+}
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scriptnonce-before-execute.js b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scriptnonce-before-execute.js
new file mode 100644
index 0000000..2676b34
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scriptnonce-before-execute.js
@@ -0,0 +1,8 @@
+// This script is executed after |scr1| and |scr2| are inserted into DOM
+// before their execution (if not blocked by CSP).
+if (document.getElementById('scr1')) {
+ document.getElementById('scr1').setAttribute('nonce', 'wrong');
+}
+if (document.getElementById('scr2')) {
+ document.getElementById('scr2').setAttribute('nonce', 'abc');
+}
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/empty.css b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/empty.css
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/empty.css
diff --git a/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/inline-script-should-be-blocked.js b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/inline-script-should-be-blocked.js
new file mode 100644
index 0000000..f32d250
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/inline-script-should-be-blocked.js
@@ -0,0 +1,14 @@
+var t;
+async_test(t => {
+ self.t = t;
+ const s = document.createElement('script');
+ s.onerror = t.step_func(function() {
+ assert_unreached('Script error event should not be fired.');
+ });
+ s.onload = t.step_func(function() {
+ assert_unreached('Script load event should not be fired.');
+ });
+ s.innerText = 'self.t.assert_unreached("Script should not run.");'
+ document.body.appendChild(s);
+ setTimeout(() => t.done(), 2000);
+});
diff --git a/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/resources/fail-to-inject-script.js b/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/resources/fail-to-inject-script.js
deleted file mode 100644
index 1f896d1..0000000
--- a/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/resources/fail-to-inject-script.js
+++ /dev/null
@@ -1,9 +0,0 @@
-var s = document.createElement('script');
-s.onerror = function() {
- done();
-};
-s.onload = function() {
- assert_unreached('Script loaded.');
-};
-document.body.appendChild(s);
-s.innerText = 'assert_unreached("Script should not run.");'
