[Trusted Types] Implement new <script> handling for Trusted Types.

This follows the proposal at
https://github.com/w3c/webappsec-trusted-types/pull/236
and effectively reverts crrev.com/c/1547746. This replaces the
(arguably rather invasive) changes in node.cc and element.cc with
more elaborate logic in html_script_element.cc. (I.e., it pushes
complexity from the super-classes into a specific subclass, at the
expense of making the sub-class do more work.)

Bug: 1026549
Change-Id: I929e9e669db7f9e6b8de5a3d0d0df661f109b644
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1924523
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723017}

third_party/blink/renderer/core/dom/element.cc

@@ -4684,7 +4684,7 @@ Node* Element::InsertAdjacent(const String& where,
                               ExceptionState& exception_state) {
   if (DeprecatedEqualIgnoringCase(where, "beforeBegin")) {
     if (ContainerNode* parent = parentNode()) {
-      parent->insertBefore(new_child, this, exception_state);
+      parent->InsertBefore(new_child, this, exception_state);
       if (!exception_state.HadException())
         return new_child;
     }
@@ -4692,18 +4692,18 @@ Node* Element::InsertAdjacent(const String& where,
   }
 
   if (DeprecatedEqualIgnoringCase(where, "afterBegin")) {
-    insertBefore(new_child, firstChild(), exception_state);
+    InsertBefore(new_child, firstChild(), exception_state);
     return exception_state.HadException() ? nullptr : new_child;
   }
 
   if (DeprecatedEqualIgnoringCase(where, "beforeEnd")) {
-    appendChild(new_child, exception_state);
+    AppendChild(new_child, exception_state);
     return exception_state.HadException() ? nullptr : new_child;
   }
 
   if (DeprecatedEqualIgnoringCase(where, "afterEnd")) {
     if (ContainerNode* parent = parentNode()) {
-      parent->insertBefore(new_child, nextSibling(), exception_state);
+      parent->InsertBefore(new_child, nextSibling(), exception_state);
       if (!exception_state.HadException())
         return new_child;
     }

third_party/blink/renderer/core/dom/node.cc

@@ -674,10 +674,6 @@ void Node::DidEndCustomizedScrollPhase() {
 Node* Node::insertBefore(Node* new_child,
                          Node* ref_child,
                          ExceptionState& exception_state) {
-  new_child = TrustedTypesCheckForScriptNode(new_child, exception_state);
-  if (!new_child)
-    return nullptr;
-
   auto* this_node = DynamicTo<ContainerNode>(this);
   if (this_node)
     return this_node->InsertBefore(new_child, ref_child, exception_state);
@@ -695,10 +691,6 @@ Node* Node::insertBefore(Node* new_child, Node* ref_child) {
 Node* Node::replaceChild(Node* new_child,
                          Node* old_child,
                          ExceptionState& exception_state) {
-  new_child = TrustedTypesCheckForScriptNode(new_child, exception_state);
-  if (!new_child)
-    return nullptr;
-
   auto* this_node = DynamicTo<ContainerNode>(this);
   if (this_node)
     return this_node->ReplaceChild(new_child, old_child, exception_state);
@@ -729,10 +721,6 @@ Node* Node::removeChild(Node* old_child) {
 }
 
 Node* Node::appendChild(Node* new_child, ExceptionState& exception_state) {
-  new_child = TrustedTypesCheckForScriptNode(new_child, exception_state);
-  if (!new_child)
-    return nullptr;
-
   auto* this_node = DynamicTo<ContainerNode>(this);
   if (this_node)
     return this_node->AppendChild(new_child, exception_state);
@@ -3292,21 +3280,6 @@ void Node::RemovedFromFlatTree() {
   GetDocument().GetStyleEngine().RemovedFromFlatTree(*this);
 }
 
-Node* Node::TrustedTypesCheckForScriptNode(
-    Node* child,
-    ExceptionState& exception_state) const {
-  DCHECK(child);
-  bool needs_check = IsA<HTMLScriptElement>(this) && child->IsTextNode() &&
-                     GetDocument().IsTrustedTypesEnabledForDoc();
-  if (!needs_check)
-    return child;
-
-  child = TrustedTypesCheckForHTMLScriptElement(child, &GetDocument(),
-                                                exception_state);
-  DCHECK_EQ(!child, exception_state.HadException());
-  return child;
-}
-
 void Node::Trace(Visitor* visitor) {
   visitor->Trace(parent_or_shadow_host_node_);
   visitor->Trace(previous_);

third_party/blink/renderer/core/dom/node.h

@@ -1063,9 +1063,6 @@ class CORE_EXPORT Node : public EventTarget {
   const HeapHashSet<Member<MutationObserverRegistration>>*
   TransientMutationObserverRegistry();
 
-  inline Node* TrustedTypesCheckForScriptNode(Node* child,
-                                              ExceptionState&) const;
-
   uint32_t node_flags_;
   Member<Node> parent_or_shadow_host_node_;
   Member<TreeScope> tree_scope_;

third_party/blink/renderer/core/html/html_script_element.cc

@@ -134,7 +134,11 @@ void HTMLScriptElement::setInnerText(
   String value = GetStringFromTrustedScript(string_or_trusted_script,
                                             &GetDocument(), exception_state);
   if (!exception_state.HadException()) {
+    // https://w3c.github.io/webappsec-trusted-types/dist/spec/#setting-slot-values
+    // On setting, the innerText [...] perform the regular steps, and then set
+    // content object's [[ScriptText]] internal slot value [...].
     HTMLElement::setInnerText(value, exception_state);
+    script_text_internal_slot_ = value;
   }
 }
 
@@ -144,7 +148,11 @@ void HTMLScriptElement::setTextContent(
   String value = GetStringFromTrustedScript(string_or_trusted_script,
                                             &GetDocument(), exception_state);
   if (!exception_state.HadException()) {
+    // https://w3c.github.io/webappsec-trusted-types/dist/spec/#setting-slot-values
+    // On setting, [..] textContent [..] perform the regular steps, and then set
+    // content object's [[ScriptText]] internal slot value [...].
     Node::setTextContent(value);
+    script_text_internal_slot_ = value;
   }
 }
 
@@ -153,6 +161,12 @@ void HTMLScriptElement::setAsync(bool async) {
   loader_->HandleAsyncAttribute();
 }
 
+void HTMLScriptElement::FinishParsingChildren() {
+  Element::FinishParsingChildren();
+  DCHECK(script_text_internal_slot_.IsEmpty());
+  script_text_internal_slot_ = TextFromChildren();
+}
+
 bool HTMLScriptElement::async() const {
   return FastHasAttribute(html_names::kAsyncAttr) || loader_->IsNonBlocking();
 }
@@ -205,6 +219,10 @@ String HTMLScriptElement::ChildTextContent() {
   return TextFromChildren();
 }
 
+String HTMLScriptElement::ScriptTextInternalSlot() const {
+  return script_text_internal_slot_;
+}
+
 bool HTMLScriptElement::AsyncAttributeValue() const {
   return FastHasAttribute(html_names::kAsyncAttr);
 }

third_party/blink/renderer/core/html/html_script_element.h

@@ -62,6 +62,8 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
 
   void Trace(Visitor*) override;
 
+  void FinishParsingChildren() override;
+
  private:
   void ParseAttribute(const AttributeModificationParams&) override;
   InsertionNotificationRequest InsertedInto(ContainerNode&) override;
@@ -73,6 +75,9 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
   bool HasLegalLinkAttribute(const QualifiedName&) const override;
   const QualifiedName& SubResourceAttributeName() const override;
 
+  void SetTrustedTextContent(const String&);
+  void SetTrustedInnerText(const String&, ExceptionState&);
+
   // ScriptElementBase overrides:
   String SourceAttributeValue() const override;
   String CharsetAttributeValue() const override;
@@ -86,6 +91,7 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
   String ReferrerPolicyAttributeValue() const override;
   String ImportanceAttributeValue() const override;
   String ChildTextContent() override;
+  String ScriptTextInternalSlot() const override;
   bool AsyncAttributeValue() const override;
   bool DeferAttributeValue() const override;
   bool HasSourceAttribute() const override;
@@ -105,6 +111,9 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
 
   Element& CloneWithoutAttributesAndChildren(Document&) const override;
 
+  // https://w3c.github.io/webappsec-trusted-types/dist/spec/#script-scripttext
+  String script_text_internal_slot_;
+
   Member<ScriptLoader> loader_;
 };
 

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

@@ -37,8 +37,8 @@ class MockScriptElementBase : public GarbageCollected<MockScriptElementBase>,
   MOCK_CONST_METHOD0(NomoduleAttributeValue, bool());
   MOCK_CONST_METHOD0(SourceAttributeValue, String());
   MOCK_CONST_METHOD0(TypeAttributeValue, String());
-
   MOCK_METHOD0(ChildTextContent, String());
+  MOCK_CONST_METHOD0(ScriptTextInternalSlot, String());
   MOCK_CONST_METHOD0(HasSourceAttribute, bool());
   MOCK_CONST_METHOD0(IsConnected, bool());
   MOCK_CONST_METHOD0(HasChildren, bool());

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

@@ -52,7 +52,11 @@ class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin {
   virtual String ReferrerPolicyAttributeValue() const = 0;
   virtual String ImportanceAttributeValue() const = 0;
 
+  // This implements https://dom.spec.whatwg.org/#concept-child-text-content
   virtual String ChildTextContent() = 0;
+  // This supports
+  // https://w3c.github.io/webappsec-trusted-types/dist/spec/#prepare-script-url-and-text
+  virtual String ScriptTextInternalSlot() const = 0;
   virtual bool HasSourceAttribute() const = 0;
   virtual bool IsConnected() const = 0;
   virtual bool HasChildren() const = 0;

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

@@ -53,6 +53,7 @@
 #include "third_party/blink/renderer/core/script/script_element_base.h"
 #include "third_party/blink/renderer/core/script/script_runner.h"
 #include "third_party/blink/renderer/core/svg_names.h"
+#include "third_party/blink/renderer/core/trustedtypes/trusted_types_util.h"
 #include "third_party/blink/renderer/platform/bindings/parkable_string.h"
 #include "third_party/blink/renderer/platform/instrumentation/histogram.h"
 #include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
@@ -289,7 +290,15 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
     non_blocking_ = true;
 
   // <spec step="4">Let source text be the element's child text content.</spec>
-  const String source_text = element_->ChildTextContent();
+  //
+  // Trusted Types additionally requires:
+  // https://w3c.github.io/webappsec-trusted-types/dist/spec/#slot-value-verification
+  // - Step 4: Execute the Prepare the script URL and text algorithm upon the
+  //     script element. If that algorithm threw an error, then return. The
+  //     script is not executed.
+  // - Step 5: Let source text be the element’s [[ScriptText]] internal slot
+  //     value.
+  const String source_text = GetScriptText();
 
   // <spec step="5">If the element has no src attribute, and source text is the
   // empty string, then return. The script is not executed.</spec>
@@ -386,8 +395,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
   // executed. [CSP]</spec>
   if (!element_->HasSourceAttribute() &&
       !element_->AllowInlineScriptForCSP(element_->GetNonceForElement(),
-                                         position.line_,
-                                         element_->ChildTextContent())) {
+                                         position.line_, source_text)) {
     return false;
   }
 
@@ -633,8 +641,8 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
       // 1. Let import map parse result be the result of create an import map
       // parse result, given source text, base URL and settings object. [spec
       // text]
-      PendingImportMap* pending_import_map = PendingImportMap::CreateInline(
-          *element_, element_->ChildTextContent(), base_url);
+      PendingImportMap* pending_import_map =
+          PendingImportMap::CreateInline(*element_, source_text, base_url);
 
       // Because we currently support inline import maps only, the pending
       // import map is ready immediately and thus we call `register an import
@@ -662,8 +670,8 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
         }
 
         prepared_pending_script_ = ClassicPendingScript::CreateInline(
-            element_, position, base_url, element_->ChildTextContent(),
-            script_location_type, options);
+            element_, position, base_url, source_text, script_location_type,
+            options);
 
         // <spec step="25.2.A.2">Set the script's script to script.</spec>
         //
@@ -689,10 +697,10 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
         // <spec label="fetch-an-inline-module-script-graph" step="1">Let script
         // be the result of creating a JavaScript module script using source
         // text, settings object, base URL, and options.</spec>
-        ModuleScript* module_script = JSModuleScript::Create(
-            ParkableString(element_->ChildTextContent().Impl()), nullptr,
-            ScriptSourceLocationType::kInline, modulator, source_url, base_url,
-            options, position);
+        ModuleScript* module_script =
+            JSModuleScript::Create(ParkableString(source_text.Impl()), nullptr,
+                                   ScriptSourceLocationType::kInline, modulator,
+                                   source_url, base_url, options, position);
 
         // <spec label="fetch-an-inline-module-script-graph" step="2">If script
         // is null, asynchronously complete this algorithm with null, and abort
@@ -1015,4 +1023,20 @@ ScriptLoader::GetPendingScriptIfControlledByScriptRunnerForCrossDocMove() {
   return pending_script_;
 }
 
+String ScriptLoader::GetScriptText() const {
+  // Step 3 of
+  // https://w3c.github.io/webappsec-trusted-types/dist/spec/#abstract-opdef-prepare-the-script-url-and-text
+  // called from § 4.1.3.3, step 4 of
+  // https://w3c.github.io/webappsec-trusted-types/dist/spec/#slot-value-verification
+  // This will return the [[ScriptText]] internal slot value after that step,
+  // or a null string if the the Trusted Type algorithm threw an error.
+  String child_text_content = element_->ChildTextContent();
+  DCHECK(!child_text_content.IsNull());
+  String script_text_internal_slot = element_->ScriptTextInternalSlot();
+  if (child_text_content == script_text_internal_slot)
+    return child_text_content;
+  return GetStringForScriptExecution(child_text_content,
+                                     element_->GetDocument().ContextDocument());
+}
+
 }  // namespace blink

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

@@ -140,6 +140,9 @@ class CORE_EXPORT ScriptLoader final : public GarbageCollected<ScriptLoader>,
   // PendingScriptClient
   void PendingScriptFinished(PendingScript*) override;
 
+  // Get the effective script text (after Trusted Types checking).
+  String GetScriptText() const;
+
   Member<ScriptElementBase> element_;
 
   // https://html.spec.whatwg.org/C/#script-processing-model

third_party/blink/renderer/core/svg/svg_script_element.cc

@@ -93,6 +93,8 @@ bool SVGScriptElement::IsURLAttribute(const Attribute& attribute) const {
 void SVGScriptElement::FinishParsingChildren() {
   SVGElement::FinishParsingChildren();
   have_fired_load_ = true;
+  DCHECK(script_text_internal_slot_.IsEmpty());
+  script_text_internal_slot_ = TextFromChildren();
 }
 
 bool SVGScriptElement::HaveLoadedRequiredResources() {
@@ -111,6 +113,10 @@ String SVGScriptElement::ChildTextContent() {
   return TextFromChildren();
 }
 
+String SVGScriptElement::ScriptTextInternalSlot() const {
+  return script_text_internal_slot_;
+}
+
 bool SVGScriptElement::HasSourceAttribute() const {
   return href()->IsSpecified();
 }

third_party/blink/renderer/core/svg/svg_script_element.h

@@ -82,6 +82,7 @@ class SVGScriptElement final : public SVGElement,
   String SourceAttributeValue() const override;
   String TypeAttributeValue() const override;
   String ChildTextContent() override;
+  String ScriptTextInternalSlot() const override;
   bool HasSourceAttribute() const override;
   bool IsConnected() const override;
   bool HasChildren() const override;
@@ -105,6 +106,8 @@ class SVGScriptElement final : public SVGElement,
 
   bool have_fired_load_ = false;
 
+  String script_text_internal_slot_;
+
   Member<ScriptLoader> loader_;
 };
 

third_party/blink/renderer/core/trustedtypes/trusted_types_util.h

@@ -13,7 +13,6 @@ namespace blink {
 class Document;
 class ExecutionContext;
 class ExceptionState;
-class Node;
 class StringOrTrustedHTML;
 class StringOrTrustedHTMLOrTrustedScriptOrTrustedScriptURL;
 class StringOrTrustedScript;
@@ -57,25 +56,19 @@ String CORE_EXPORT GetStringFromTrustedScript(StringOrTrustedScript,
                                               const ExecutionContext*,
                                               ExceptionState&);
 
-String GetStringFromTrustedScript(const String&,
-                                  const ExecutionContext*,
-                                  ExceptionState&);
+String CORE_EXPORT GetStringFromTrustedScript(const String&,
+                                              const ExecutionContext*,
+                                              ExceptionState&);
 
 String CORE_EXPORT GetStringFromTrustedScriptURL(StringOrTrustedScriptURL,
                                                  const ExecutionContext*,
                                                  ExceptionState&);
 
-// For <script> elements, we need to treat insertion of DOM text nodes
-// as equivalent to string assignment. This checks the child-node to be
-// inserted and runs all of the Trusted Types checks if it's a text node.
-//
-// Returns nullptr if the check failed, or the node to use (possibly child)
-//         if they succeeded.
-Node* TrustedTypesCheckForHTMLScriptElement(Node* child,
-                                            Document*,
-                                            ExceptionState&);
-
+// Functionally equivalent to GetStringFromTrustedScript(const String&, ...),
+// but with setup & error handling suitable for the asynchronous execution
+// cases.
 String TrustedTypesCheckForJavascriptURLinNavigation(const String&, Document*);
+String CORE_EXPORT GetStringForScriptExecution(const String&, Document*);
 
 // Determine whether a Trusted Types check is needed in this execution context.
 //