[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}

third_party/blink/renderer/core/script/classic_script.h

@@ -36,9 +36,6 @@ class CORE_EXPORT ClassicScript final : public Script {
   }
   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_;

third_party/blink/renderer/core/script/js_module_script.cc

@@ -57,8 +57,8 @@ JSModuleScript* JSModuleScript::Create(
   // 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 @@ JSModuleScript* JSModuleScript::CreateForTest(
     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 @@ JSModuleScript* JSModuleScript::CreateInternal(
   // <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 @@ JSModuleScript::JSModuleScript(Modulator* settings_object,
                                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 @@ JSModuleScript::JSModuleScript(Modulator* settings_object,
                    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 @@ void JSModuleScript::ProduceCache() {
   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;

third_party/blink/renderer/core/script/js_module_script.h

@@ -50,7 +50,7 @@ class CORE_EXPORT JSModuleScript final : public ModuleScript,
                  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 @@ class CORE_EXPORT JSModuleScript final : public ModuleScript,
   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 @@ class CORE_EXPORT JSModuleScript final : public ModuleScript,
       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_;
 

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 @@ void PendingScript::ExecuteScriptBlock(const KURL& document_url) {
 
   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();

third_party/blink/renderer/core/script/script.h

@@ -35,9 +35,6 @@ class CORE_EXPORT Script : public GarbageCollectedFinalized<Script> {
   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_; }
 

third_party/blink/renderer/core/script/script_loader.cc

@@ -415,7 +415,19 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
   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;
 

third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.cc

@@ -148,17 +148,6 @@ v8::MaybeLocal<v8::Value> ValueWrapperSyntheticModuleScript::EvaluationSteps(
   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);

third_party/blink/renderer/core/script/value_wrapper_synthetic_module_script.h

@@ -65,7 +65,6 @@ class CORE_EXPORT ValueWrapperSyntheticModuleScript final
       v8::Local<v8::Context> context,
       v8::Local<v8::Module> module);
 
-  String InlineSourceTextForCSP() const override;
   void Trace(blink::Visitor* visitor) override;
 
  private:

third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/1.1/scripthash-basic-blocked-error-event.html → 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>

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-1.html

+<!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>

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scripthash-changed-2.html

+<!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>

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-1.html

+<!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>

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/scriptnonce-changed-2.html

+<!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>

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scripthash-before-execute.js

+// 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';";
+}

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/change-scriptnonce-before-execute.js

+// 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');
+}

third_party/blink/web_tests/external/wpt/content-security-policy/script-src/support/inline-script-should-be-blocked.js

+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);
+});

third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/resources/fail-to-inject-script.js

-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.");'