Bug 1397308 - Implement CSP 'Is element nonceable?' check. r=emilio,hsivonen,freddyb

Differential Revision: https://phabricator.services.mozilla.com/D198150

dom/base/Element.h

@@ -187,8 +187,15 @@ enum : uint32_t {
   // it's not going to be considered again.
   ELEMENT_PROCESSED_BY_LCP_FOR_TEXT = ELEMENT_FLAG_BIT(5),
 
+  // If this flag is set on an element, this means the HTML parser encountered
+  // a duplicate attribute error:
+  // https://html.spec.whatwg.org/multipage/parsing.html#parse-error-duplicate-attribute
+  // This flag is used for detecting dangling markup attacks in the CSP
+  // algorithm https://w3c.github.io/webappsec-csp/#is-element-nonceable.
+  ELEMENT_PARSER_HAD_DUPLICATE_ATTR_ERROR = ELEMENT_FLAG_BIT(6),
+
   // Remaining bits are for subclasses
-  ELEMENT_TYPE_SPECIFIC_BITS_OFFSET = NODE_TYPE_SPECIFIC_BITS_OFFSET + 6
+  ELEMENT_TYPE_SPECIFIC_BITS_OFFSET = NODE_TYPE_SPECIFIC_BITS_OFFSET + 7
 };
 
 #undef ELEMENT_FLAG_BIT
@@ -1720,6 +1727,10 @@ class Element : public FragmentOrElement {
    */
   void TryReserveAttributeCount(uint32_t aAttributeCount);
 
+  void SetParserHadDuplicateAttributeError() {
+    SetFlags(ELEMENT_PARSER_HAD_DUPLICATE_ATTR_ERROR);
+  }
+
   /**
    * Set a content attribute via a reflecting nullable string IDL
    * attribute (e.g. a CORS attribute).  If DOMStringIsNull(aValue),

dom/interfaces/security/nsIContentSecurityPolicy.idl

@@ -128,7 +128,7 @@ interface nsIContentSecurityPolicy : nsISerializable
    *                           STYLE_SRC_(ELEM|ATTR)_DIRECTIVE.
    * @param aHasUnsafeHash Only hash this when the 'unsafe-hashes' directive is
    *                       also specified.
-   * @param aNonce The nonce string to check against the policy
+   * @param aNonce The nonce string to check against the policy.
    * @param aParserCreated If the script element was created by the HTML Parser
    * @param aTriggeringElement The script element of the inline resource to
    *        hash. It can be null.

dom/script/ScriptLoader.cpp

@@ -66,6 +66,7 @@
 #include "nsIPrincipal.h"
 #include "nsJSPrincipals.h"
 #include "nsContentPolicyUtils.h"
+#include "nsContentSecurityUtils.h"
 #include "nsIClassifiedChannel.h"
 #include "nsIHttpChannel.h"
 #include "nsIHttpChannelInternal.h"
@@ -1115,12 +1116,8 @@ bool ScriptLoader::ProcessExternalScript(nsIScriptElement* aElement,
     return false;
   }
 
-  nsAutoString nonce;
-  if (nsString* cspNonce = static_cast<nsString*>(
-          aScriptContent->GetProperty(nsGkAtoms::nonce))) {
-    nonce = *cspNonce;
-  }
-
+  nsString nonce = nsContentSecurityUtils::GetIsElementNonceableNonce(
+      *aScriptContent->AsElement());
   SRIMetadata sriMetadata;
   {
     nsAutoString integrity;
@@ -1326,12 +1323,8 @@ bool ScriptLoader::ProcessInlineScript(nsIScriptElement* aElement,
     return false;
   }
 
-  nsCOMPtr<nsINode> node = do_QueryInterface(aElement);
-  nsAutoString nonce;
-  if (nsString* cspNonce =
-          static_cast<nsString*>(node->GetProperty(nsGkAtoms::nonce))) {
-    nonce = *cspNonce;
-  }
+  nsCOMPtr<Element> element = do_QueryInterface(aElement);
+  nsString nonce = nsContentSecurityUtils::GetIsElementNonceableNonce(*element);
 
   // Does CSP allow this inline script to run?
   if (!CSPAllowsInlineScript(aElement, nonce, mDocument)) {
@@ -1374,6 +1367,8 @@ bool ScriptLoader::ProcessInlineScript(nsIScriptElement* aElement,
   ReferrerPolicy referrerPolicy = GetReferrerPolicy(aElement);
   ParserMetadata parserMetadata = GetParserMetadata(aElement);
 
+  // NOTE: The `nonce` as specified here is significant, because it's inherited
+  // by other scripts (e.g. modules created via dynamic imports).
   RefPtr<ScriptLoadRequest> request =
       CreateLoadRequest(aScriptKind, mDocument->GetDocumentURI(), aElement,
                         mDocument->NodePrincipal(), corsMode, nonce,

dom/security/nsCSPContext.cpp

@@ -9,6 +9,7 @@
 
 #include "nsCOMPtr.h"
 #include "nsContentPolicyUtils.h"
+#include "nsContentSecurityUtils.h"
 #include "nsContentUtils.h"
 #include "nsCSPContext.h"
 #include "nsCSPParser.h"
@@ -593,11 +594,12 @@ nsCSPContext::GetAllowsInline(CSPDirective aDirective, bool aHasUnsafeHash,
   }
 
   EnsureIPCPoliciesRead();
-  nsAutoString content(u""_ns);
+  nsAutoString content;
 
   // always iterate all policies, otherwise we might not send out all reports
   for (uint32_t i = 0; i < mPolicies.Length(); i++) {
     // https://w3c.github.io/webappsec-csp/#match-element-to-source-list
+
     // Step 1. If §6.7.3.2 Does a source list allow all inline behavior for
     // type? returns "Allows" given list and type, return "Matches".
     if (mPolicies[i]->allowsAllInlineBehavior(aDirective)) {
@@ -605,10 +607,24 @@ nsCSPContext::GetAllowsInline(CSPDirective aDirective, bool aHasUnsafeHash,
     }
 
     // Step 2. If type is "script" or "style", and §6.7.3.1 Is element
-    // nonceable? returns "Nonceable" when executed upon element: [...]
-    // TODO(Bug 1397308) Implement "is element nonceable?" CSP checks
-    if (mPolicies[i]->allows(aDirective, CSP_NONCE, aNonce)) {
-      continue;
+    // nonceable? returns "Nonceable" when executed upon element:
+    if ((aDirective == SCRIPT_SRC_ELEM_DIRECTIVE ||
+         aDirective == STYLE_SRC_ELEM_DIRECTIVE) &&
+        aTriggeringElement && !aNonce.IsEmpty()) {
+#ifdef DEBUG
+      // NOTE: Folllowing Chrome "Is element nonceable?" doesn't apply to
+      // <style>.
+      if (aDirective == SCRIPT_SRC_ELEM_DIRECTIVE) {
+        // Our callers should have checked this.
+        MOZ_ASSERT(nsContentSecurityUtils::GetIsElementNonceableNonce(
+                       *aTriggeringElement) == aNonce);
+      }
+#endif
+
+      // Step 2.1. For each expression of list: [...]
+      if (mPolicies[i]->allows(aDirective, CSP_NONCE, aNonce)) {
+        continue;
+      }
     }
 
     // Check the content length to ensure the content is not allocated more than

dom/security/nsContentSecurityUtils.cpp

@@ -1190,6 +1190,64 @@ bool nsContentSecurityUtils::CheckCSPFrameAncestorAndXFO(nsIChannel* aChannel) {
                                            isFrameOptionsIgnored);
 }
 
+// https://w3c.github.io/webappsec-csp/#is-element-nonceable
+/* static */
+nsString nsContentSecurityUtils::GetIsElementNonceableNonce(
+    const Element& aElement) {
+  // Step 1. If element does not have an attribute named "nonce", return "Not
+  // Nonceable".
+  nsString nonce;
+  if (nsString* cspNonce =
+          static_cast<nsString*>(aElement.GetProperty(nsGkAtoms::nonce))) {
+    nonce = *cspNonce;
+  }
+  if (nonce.IsEmpty()) {
+    return nonce;
+  }
+
+  // Step 2. If element is a script element, then for each attribute of
+  // element’s attribute list:
+  if (nsCOMPtr<nsIScriptElement> script =
+          do_QueryInterface(const_cast<Element*>(&aElement))) {
+    auto containsScriptOrStyle = [](const nsAString& aStr) {
+      return aStr.LowerCaseFindASCII("<script") != kNotFound ||
+             aStr.LowerCaseFindASCII("<style") != kNotFound;
+    };
+
+    nsString value;
+    uint32_t i = 0;
+    while (BorrowedAttrInfo info = aElement.GetAttrInfoAt(i++)) {
+      // Step 2.1. If attribute’s name contains an ASCII case-insensitive match
+      // for "<script" or "<style", return "Not Nonceable".
+      const nsAttrName* name = info.mName;
+      if (nsAtom* prefix = name->GetPrefix()) {
+        if (containsScriptOrStyle(nsDependentAtomString(prefix))) {
+          return EmptyString();
+        }
+      }
+      if (containsScriptOrStyle(nsDependentAtomString(name->LocalName()))) {
+        return EmptyString();
+      }
+
+      // Step 2.2. If attribute’s value contains an ASCII case-insensitive match
+      // for "<script" or "<style", return "Not Nonceable".
+      info.mValue->ToString(value);
+      if (containsScriptOrStyle(value)) {
+        return EmptyString();
+      }
+    }
+  }
+
+  // Step 3. If element had a duplicate-attribute parse error during
+  // tokenization, return "Not Nonceable".
+  if (aElement.HasFlag(ELEMENT_PARSER_HAD_DUPLICATE_ATTR_ERROR)) {
+    return EmptyString();
+  }
+
+  // Step 4. Return "Nonceable".
+  return nonce;
+}
+
 #if defined(DEBUG)
 /* static */
 void nsContentSecurityUtils::AssertAboutPageHasCSP(Document* aDocument) {

dom/security/nsContentSecurityUtils.h

@@ -21,6 +21,7 @@ class NS_ConvertUTF8toUTF16;
 
 namespace mozilla::dom {
 class Document;
+class Element;
 }  // namespace mozilla::dom
 
 using FilenameTypeAndDetails = std::pair<nsCString, mozilla::Maybe<nsString>>;
@@ -66,6 +67,13 @@ class nsContentSecurityUtils {
   // 2. x-frame-options
   static bool CheckCSPFrameAncestorAndXFO(nsIChannel* aChannel);
 
+  // Implements https://w3c.github.io/webappsec-csp/#is-element-nonceable.
+  //
+  // Returns an empty nonce for elements without a nonce OR when a potential
+  // dangling markup attack was detected.
+  static nsString GetIsElementNonceableNonce(
+      const mozilla::dom::Element& aElement);
+
   // Helper function to Check if a Download is allowed;
   static long ClassifyDownload(nsIChannel* aChannel,
                                const nsAutoCString& aMimeTypeGuess);

layout/style/nsStyleUtil.cpp

@@ -363,7 +363,7 @@ bool nsStyleUtil::CSPAllowsInlineStyle(
   }
 
   bool isStyleElement = false;
-  // query the nonce
+  // Query the nonce.
   nsAutoString nonce;
   if (aElement && aElement->NodeInfo()->NameAtom() == nsGkAtoms::style) {
     isStyleElement = true;

parser/html/nsHtml5HtmlAttributes.cpp

@@ -135,6 +135,7 @@ void nsHtml5HtmlAttributes::clear(int32_t aMode) {
   }
   mStorage.TruncateLength(0);
   mMode = aMode;
+  mDuplicateAttributeError = false;
 }
 
 void nsHtml5HtmlAttributes::releaseValue(int32_t aIndex) {

parser/html/nsHtml5HtmlAttributes.h

@@ -56,12 +56,16 @@ class nsHtml5HtmlAttributes {
  private:
   AutoTArray<nsHtml5AttributeEntry, 5> mStorage;
   int32_t mMode;
+  bool mDuplicateAttributeError = false;
   void AddEntry(nsHtml5AttributeEntry&& aEntry);
 
  public:
   explicit nsHtml5HtmlAttributes(int32_t aMode);
   ~nsHtml5HtmlAttributes();
 
+  void setDuplicateAttributeError() { mDuplicateAttributeError = true; }
+  bool getDuplicateAttributeError() { return mDuplicateAttributeError; }
+
   // Remove getIndex when removing isindex support
   int32_t getIndex(nsHtml5AttributeName* aName);
 

parser/html/nsHtml5TokenizerCppSupplement.h

@@ -432,6 +432,12 @@ void nsHtml5Tokenizer::errNcrUnassigned() {
 }
 
 void nsHtml5Tokenizer::errDuplicateAttribute() {
+  if (attributes) {
+    // There is an open issue for properly specifying this:
+    // https://github.com/whatwg/html/issues/3257
+    attributes->setDuplicateAttributeError();
+  }
+
   if (MOZ_UNLIKELY(mViewSource)) {
     mViewSource->AddErrorToCurrentNode("errDuplicateAttribute");
   }

parser/html/nsHtml5TreeOperation.cpp

@@ -416,6 +416,9 @@ void nsHtml5TreeOperation::SetHTMLElementAttributes(
     Element* aElement, nsAtom* aName, nsHtml5HtmlAttributes* aAttributes) {
   int32_t len = aAttributes->getLength();
   aElement->TryReserveAttributeCount((uint32_t)len);
+  if (aAttributes->getDuplicateAttributeError()) {
+    aElement->SetParserHadDuplicateAttributeError();
+  }
   for (int32_t i = 0; i < len; i++) {
     nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);
     nsAtom* klass = val.MaybeAsAtom();
@@ -541,6 +544,10 @@ nsIContent* nsHtml5TreeOperation::CreateSVGElement(
     return newContent;
   }
 
+  if (aAttributes->getDuplicateAttributeError()) {
+    newContent->SetParserHadDuplicateAttributeError();
+  }
+
   int32_t len = aAttributes->getLength();
   for (int32_t i = 0; i < len; i++) {
     nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);
@@ -589,6 +596,10 @@ nsIContent* nsHtml5TreeOperation::CreateMathMLElement(
     return newContent;
   }
 
+  if (aAttributes->getDuplicateAttributeError()) {
+    newContent->SetParserHadDuplicateAttributeError();
+  }
+
   int32_t len = aAttributes->getLength();
   for (int32_t i = 0; i < len; i++) {
     nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);

testing/web-platform/meta/content-security-policy/script-src/nonce-enforce-blocked.html.ini

@@ -1,5 +0,0 @@
-[nonce-enforce-blocked.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [Unnonced scripts generate reports.]
-    expected: FAIL

testing/web-platform/tests/content-security-policy/script-src/nonce-enforce-blocked.html

@@ -5,7 +5,7 @@
 <script nonce="abc">
     var t = async_test("Unnonced scripts generate reports.");
     var events = 0;
-    var firstLine = 38;
+    var firstLine = 43;
     var expectations = {}
     expectations[firstLine] = true;
     expectations[firstLine + 3] = true;
@@ -14,11 +14,16 @@
     expectations[firstLine + 12] = true;
     expectations[firstLine + 15] = true;
     expectations[firstLine + 18] = true;
+    expectations[firstLine + 21] = true;
+    expectations[firstLine + 24] = true;
+    expectations[firstLine + 28] = true;
     expectations["/content-security-policy/support/nonce-should-be-blocked.js?1"] = true;
     expectations["/content-security-policy/support/nonce-should-be-blocked.js?2"] = true;
     expectations["/content-security-policy/support/nonce-should-be-blocked.js?3"] = true;
     expectations["/content-security-policy/support/nonce-should-be-blocked.js?4"] = true;
     expectations["/content-security-policy/support/nonce-should-be-blocked.js?5"] = true;
+    expectations["/content-security-policy/support/nonce-should-be-blocked.js?6"] = true;
+    expectations["/content-security-policy/support/nonce-should-be-blocked.js?7"] = true;
 
     document.addEventListener('securitypolicyviolation', t.step_func(e => {
         if (e.lineNumber) {
@@ -31,7 +36,7 @@
             assert_true(expectations[url.pathname + url.search], "URL: " + e.blockedURI);
         }
         events++;
-        if (events == 12)
+        if (events == Object.keys(expectations).length)
           t.done();
     }));
 </script>
@@ -47,17 +52,30 @@
 <script attribute<script nonce="abc">
     t.unreached_func("'attribute<script', no execution.")();
 </script>
+<script attribute<style="value" nonce="abc">
+    t.unreached_func("'attribute<style' attribute, no execution.")();
+</script>
 <script attribute=<script nonce="abc">
     t.unreached_func("'<script' value, no execution.")();
 </script>
 <script attribute=value<script nonce="abc">
     t.unreached_func("'value<script', no execution.")();
 </script>
-<script attribute="" attribute=<style nonce="abc">
+<script attribute attribute nonce="abc">
     t.unreached_func("Duplicate attribute, no execution.")();
 </script>
+<script attribute attribute=<style nonce="abc">
+    t.unreached_func("2# Duplicate attribute, no execution.")();
+</script>
+<svg xmlns="http://www.w3.org/2000/svg">
+<script attribute attribute nonce="abc">
+    t.unreached_func("Duplicate attribute in SVG, no execution.")();
+</script>
+</svg>
 <script src="../support/nonce-should-be-blocked.js?1" <script nonce="abc"></script>
 <script src="../support/nonce-should-be-blocked.js?2" attribute=<script nonce="abc"></script>
 <script src="../support/nonce-should-be-blocked.js?3" <style nonce="abc"></script>
 <script src="../support/nonce-should-be-blocked.js?4" attribute=<style nonce="abc"></script>
-<script src="../support/nonce-should-be-blocked.js?5" attribute=<style nonce="abc"></script>
+<script src="../support/nonce-should-be-blocked.js?5" attribute attribute nonce="abc"></script>
+<script src="../support/nonce-should-be-blocked.js?6" attribute<script nonce="abc"></script>
+<script src="../support/nonce-should-be-blocked.js?7" attribute=value<script nonce="abc"></script>