Issue #2466 - Part 2: Implement script-src-elem and script-src-attr

dom/events/EventListenerManager.cpp

@@ -796,7 +796,7 @@ EventListenerManager::SetEventHandler(nsIAtom* aName,
 
     if (csp) {
       bool allowsInlineScript = true;
-      rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE,
+      rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE,
                                 EmptyString(), // aNonce
                                 true, // aParserCreated (true because attribute event handler)
                                 aBody,

dom/interfaces/security/nsIContentSecurityPolicy.idl

@@ -60,6 +60,8 @@ interface nsIContentSecurityPolicy : nsISerializable
   const unsigned short REQUIRE_SRI_FOR                = 20;
   const unsigned short SANDBOX_DIRECTIVE              = 21;
   const unsigned short WORKER_SRC_DIRECTIVE           = 22;
+  const unsigned short SCRIPT_SRC_ELEM_DIRECTIVE      = 23;
+  const unsigned short SCRIPT_SRC_ATTR_DIRECTIVE      = 24;
 
   /**
    * Accessor method for a read-only string version of the policy at a given

dom/jsurl/nsJSProtocolHandler.cpp

@@ -179,8 +179,10 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel,
     rv = principal->GetCsp(getter_AddRefs(csp));
     NS_ENSURE_SUCCESS(rv, rv);
     if (csp) {
+        // javascript: is a "navigation" type, so script-src-elem applies.
+        // https://w3c.github.io/webappsec-csp/#effective-directive-for-inline-check
         bool allowsInlineScript = true;
-        rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE,
+        rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE,
                                   EmptyString(), // aNonce
                                   true,         // aParserCreated
                                   EmptyString(), // aContent

dom/locales/en-US/chrome/security/csp.properties

@@ -35,17 +35,22 @@ ignoringDuplicateSrc = Ignoring duplicate source %1$S
 # LOCALIZATION NOTE (ignoringSrcFromMetaCSP):
 # %1$S defines the ignored src
 ignoringSrcFromMetaCSP = Ignoring source ‘%1$S’ (Not supported when delivered via meta element).
-# LOCALIZATION NOTE (ignoringSrcWithinScriptStyleSrc):
+# LOCALIZATION NOTE (ignoringSrcWithinNonceOrHashDirective):
+# %1$S is the ignored src (e.g. "unsafe-inline")
+# %2$S is the directive (e.g. "script-src-elem")
+ignoringSrcWithinNonceOrHashDirective = Ignoring “%1$S” within %2$S: nonce-source or hash-source specified
+# LOCALIZATION NOTE (ignoringScriptSrcForStrictDynamic):
 # %1$S is the ignored src
-# script-src and style-src are directive names and should not be localized
+# %2$S is the directive src (e.g. "script-src-elem")
-ignoringSrcWithinScriptStyleSrc = Ignoring “%1$S” within script-src or style-src: nonce-source or hash-source specified
+# 'strict-dynamic' should not be localized
-# LOCALIZATION NOTE (ignoringSrcForStrictDynamic):
+ignoringScriptSrcForStrictDynamic = Ignoring “%1$S” within %2$S: ‘strict-dynamic’ specified
-# %1$S is the ignored src
-# script-src, as well as 'strict-dynamic' should not be localized
-ignoringSrcForStrictDynamic = Ignoring “%1$S” within script-src: ‘strict-dynamic’ specified
 # LOCALIZATION NOTE (ignoringStrictDynamic):
 # %1$S is the ignored src
 ignoringStrictDynamic = Ignoring source “%1$S” (Only supported within script-src). 
+# LOCALIZATION NOTE (ignoringUnsafeEval):
+# %1$S is the csp directive (e.g. script-src-elem)
+# 'unsafe-eval' and 'wasm-unsafe-eval' should not be localized
+ignoringUnsafeEval = Ignoring ‘unsafe-eval’ or ‘wasm-unsafe-eval’ inside “%1$S”.
 # LOCALIZATION NOTE (strictDynamicButNoHashOrNonce):
 # %1$S is the csp directive that contains 'strict-dynamic'
 # 'strict-dynamic' should not be localized

dom/script/ScriptLoader.cpp

@@ -1471,7 +1471,7 @@ CSPAllowsInlineScript(nsIScriptElement *aElement, nsIDocument *aDocument)
   aElement->GetScriptText(scriptText);
 
   bool allowInlineScript = false;
-  rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE,
+  rv = csp->GetAllowsInline(nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE,
                             nonce, parserCreated, scriptText,
                             aElement->GetScriptLineNumber(),
                             aElement->GetScriptColumnNumber(),

dom/security/nsCSPContext.cpp

@@ -499,12 +499,14 @@ nsCSPContext::reportInlineViolation(CSPDirective aDirective,
   // let's report the hash error; no need to report the unsafe-inline error
   // anymore.
   if (!aNonce.IsEmpty()) {
-    observerSubject = (aDirective == SCRIPT_SRC_DIRECTIVE)
+    observerSubject = (aDirective == SCRIPT_SRC_ELEM_DIRECTIVE ||
+                       aDirective == SCRIPT_SRC_ATTR_DIRECTIVE)
                       ? NS_LITERAL_STRING(SCRIPT_NONCE_VIOLATION_OBSERVER_TOPIC)
                       : NS_LITERAL_STRING(STYLE_NONCE_VIOLATION_OBSERVER_TOPIC);
   }
   else {
-    observerSubject = (aDirective == SCRIPT_SRC_DIRECTIVE)
+    observerSubject = (aDirective == SCRIPT_SRC_ELEM_DIRECTIVE ||
+                       aDirective == SCRIPT_SRC_ATTR_DIRECTIVE)
                       ? NS_LITERAL_STRING(SCRIPT_HASH_VIOLATION_OBSERVER_TOPIC)
                       : NS_LITERAL_STRING(STYLE_HASH_VIOLATION_OBSERVER_TOPIC);
   }
@@ -565,8 +567,11 @@ nsCSPContext::GetAllowsInline(CSPDirective aDirective,
 {
   *outAllowsInline = true;
 
-  if (aDirective != SCRIPT_SRC_DIRECTIVE && aDirective != STYLE_SRC_DIRECTIVE) {
+  if (aDirective != SCRIPT_SRC_ELEM_DIRECTIVE &&
-    MOZ_ASSERT(false, "can only allow inline for script or style");
+      aDirective != SCRIPT_SRC_ATTR_DIRECTIVE &&
+      aDirective != STYLE_SRC_DIRECTIVE) {
+    MOZ_ASSERT(false,
+               "can only allow inline for script-src-(attr/elem) or style");
     return NS_OK;
   }
 

dom/security/nsCSPParser.cpp

@@ -108,6 +108,7 @@ nsCSPParser::nsCSPParser(cspTokens& aTokens,
  : mCurChar(nullptr)
  , mEndChar(nullptr)
  , mHasHashOrNonce(false)
+ , mHasAnyUnsafeEval(false)
  , mStrictDynamic(false)
  , mUnsafeInlineKeywordSrc(nullptr)
  , mChildSrc(nullptr)
@@ -497,7 +498,9 @@ nsCSPParser::keywordSource()
     if (!sStrictDynamicEnabled) {
       return nullptr;
     }
-    if (!CSP_IsDirective(mCurDir[0], nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE)) {
+    if (!CSP_IsDirective(mCurDir[0], nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE) &&
+        !CSP_IsDirective(mCurDir[0], nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE) &&
+        !CSP_IsDirective(mCurDir[0], nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE)) {
       // Todo: Enforce 'strict-dynamic' within default-src; see Bug 1313937
       const char16_t* params[] = { u"strict-dynamic" };
       logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringStrictDynamic",
@@ -534,6 +537,7 @@ nsCSPParser::keywordSource()
     if (doc) {
       doc->SetHasUnsafeEvalCSP(true);
     }
+    mHasAnyUnsafeEval = true;
     return new nsCSPKeywordSrc(CSP_KeywordToEnum(mCurToken));
   }
   return nullptr;
@@ -1077,7 +1081,8 @@ nsCSPParser::directiveName()
   }
 
   // if we have a script-src, cache it as a fallback for worker-src
-  // in case child-src is not present
+  // in case child-src is not present. It is also used as a fallback for
+  // script-src-elem and script-src-attr.
   if (CSP_IsDirective(mCurToken, nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE)) {
     mScriptSrc = new nsCSPScriptSrcDirective(CSP_StringToCSPDirective(mCurToken));
     return mScriptSrc;
@@ -1181,6 +1186,7 @@ nsCSPParser::directive()
   // make sure to reset cache variables when trying to invalidate unsafe-inline;
   // unsafe-inline might not only appear in script-src, but also in default-src
   mHasHashOrNonce = false;
+  mHasAnyUnsafeEval = false;
   mStrictDynamic = false;
   mUnsafeInlineKeywordSrc = nullptr;
 
@@ -1200,8 +1206,12 @@ nsCSPParser::directive()
 
   // If policy contains 'strict-dynamic' invalidate all srcs within script-src.
   if (mStrictDynamic) {
-    MOZ_ASSERT(cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE),
+    MOZ_ASSERT(
-               "strict-dynamic only allowed within script-src");
+        cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE) ||
+            cspDir->equals(
+                nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE) ||
+            cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE),
+        "strict-dynamic only allowed within script-src(-elem|attr)");
     for (uint32_t i = 0; i < srcs.Length(); i++) {
       // Please note that nsCSPNonceSrc as well as nsCSPHashSrc overwrite invalidate(),
       // so it's fine to just call invalidate() on all srcs. Please also note that
@@ -1220,8 +1230,8 @@ nsCSPParser::directive()
           !StringBeginsWith(NS_ConvertUTF16toUTF8(srcStr), NS_LITERAL_CSTRING("'nonce-")) &&
           !StringBeginsWith(NS_ConvertUTF16toUTF8(srcStr), NS_LITERAL_CSTRING("'sha")))
       {
-        const char16_t* params[] = { srcStr.get() };
+        const char16_t* params[] = { srcStr.get(), mCurDir[0].get() };
-        logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringSrcForStrictDynamic",
+        logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringScriptSrcForStrictDynamic",
                                  params, ArrayLength(params));
       }
     }
@@ -1235,11 +1245,22 @@ nsCSPParser::directive()
   }
   else if (mHasHashOrNonce && mUnsafeInlineKeywordSrc &&
            (cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE) ||
+            cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE) ||
+            cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE) ||
             cspDir->equals(nsIContentSecurityPolicy::STYLE_SRC_DIRECTIVE))) {
     mUnsafeInlineKeywordSrc->invalidate();
-    // log to the console that unsafe-inline will be ignored
+    // log to the console that unsafe-inline will be ignored.
-    const char16_t* params[] = { u"'unsafe-inline'" };
+    const char16_t* params[] = { u"'unsafe-inline'", mCurDir[0].get() };
-    logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringSrcWithinScriptStyleSrc",
+    logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringSrcWithinNonceOrHashDirective",
+                             params, ArrayLength(params));
+  }
+
+  if (mHasAnyUnsafeEval &&
+      (cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE) ||
+       cspDir->equals(nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE))) {
+    // Log to the console that (wasm-)unsafe-eval will be ignored.
+    const char16_t* params[] = { mCurDir[0].get() };
+    logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringUnsafeEval",
                              params, ArrayLength(params));
   }
 
@@ -1265,13 +1286,13 @@ nsCSPParser::policy()
 
   if (mChildSrc) {
     if (!mFrameSrc) {
-      // if frame-src is specified explicitly for that policy than child-src should
+      // if frame-src is specified explicitly for that policy, then child-src should
-      // not restrict frames; if not, than child-src needs to restrict frames.
+      // not restrict frames; if not, then child-src needs to restrict frames.
       mChildSrc->setRestrictFrames();
     }
     if (!mWorkerSrc) {
-      // if worker-src is specified explicitly for that policy than child-src should
+      // if worker-src is specified explicitly for that policy, then child-src should
-      // not restrict workers; if not, than child-src needs to restrict workers.
+      // not restrict workers; if not, then child-src needs to restrict workers.
       mChildSrc->setRestrictWorkers();
     }
   }
@@ -1281,6 +1302,18 @@ nsCSPParser::policy()
     mScriptSrc->setRestrictWorkers();
   }
 
+  // If script-src is specified and script-src-elem is not specified, then
+  // script-src has to govern script requests and script blocks.
+  if (mScriptSrc && !mPolicy->hasDirective(nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE)) {
+    mScriptSrc->setRestrictScriptElem();
+  }
+
+  // If script-src is specified and script-src-attr is not specified, then
+  // script-src has to govern script attr (event handlers).
+  if (mScriptSrc && !mPolicy->hasDirective(nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE)) {
+    mScriptSrc->setRestrictScriptAttr();
+  }
+
   return mPolicy;
 }
 

dom/security/nsCSPParser.h

@@ -239,8 +239,9 @@ class nsCSPParser {
 
     // helpers to allow invalidation of srcs within script-src and style-src
     // if either 'strict-dynamic' or at least a hash or nonce is present.
-    bool               mHasHashOrNonce; // false, if no hash or nonce is defined
+    bool               mHasHashOrNonce;    // false, if no hash or nonce is defined
-    bool               mStrictDynamic;  // false, if 'strict-dynamic' is not defined
+    bool               mHasAnyUnsafeEval;  // false, if no (wasm-)unsafe-eval keyword is used.
+    bool               mStrictDynamic;     // false, if 'strict-dynamic' is not defined
     nsCSPKeywordSrc*   mUnsafeInlineKeywordSrc; // null, otherwise invlidate()
 
     // cache variables for child-src, frame-src and worker-src handling;

dom/security/nsCSPUtils.cpp

@@ -199,6 +199,10 @@ CSP_LogLocalizedStr(const char16_t* aName,
 }
 
 /* ===== Helpers ============================ */
+// This implements
+// https://w3c.github.io/webappsec-csp/#effective-directive-for-a-request.
+// However the spec doesn't currently cover all request destinations, which
+// we roughly represent using nsContentPolicyType.
 CSPDirective
 CSP_ContentTypeToDirective(nsContentPolicyType aType)
 {
@@ -213,7 +217,11 @@ CSP_ContentTypeToDirective(nsContentPolicyType aType)
     case nsIContentPolicy::TYPE_INTERNAL_SCRIPT:
     case nsIContentPolicy::TYPE_INTERNAL_SCRIPT_PRELOAD:
     case nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS:
-      return nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE;
+      // (https://github.com/w3c/webappsec-csp/issues/554)
+      // Some of these types are not explicitly defined in the spec.
+      //
+      // Chrome seems to use script-src-elem for worklet!
+      return nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE;
 
     case nsIContentPolicy::TYPE_STYLESHEET:
       return nsIContentSecurityPolicy::STYLE_SRC_DIRECTIVE;
@@ -1217,6 +1225,16 @@ nsCSPDirective::toDomCSPStruct(mozilla::dom::CSP& outCSP) const
       outCSP.mWorker_src.Value() = mozilla::Move(srcs);
       return;
 
+    case nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE:
+      outCSP.mScript_src_elem.Construct();
+      outCSP.mScript_src_elem.Value() = mozilla::Move(srcs);
+      return;
+
+    case nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE:
+      outCSP.mScript_src_attr.Construct();
+      outCSP.mScript_src_attr.Value() = mozilla::Move(srcs);
+      return;
+
     // REFERRER_DIRECTIVE and REQUIRE_SRI_FOR are handled in nsCSPPolicy::toDomCSPStruct()
 
     default:
@@ -1290,6 +1308,8 @@ bool nsCSPChildSrcDirective::equals(CSPDirective aDirective) const
 nsCSPScriptSrcDirective::nsCSPScriptSrcDirective(CSPDirective aDirective)
   : nsCSPDirective(aDirective)
   , mRestrictWorkers(false)
+  , mRestrictScriptElem(false)
+  , mRestrictScriptAttr(false)
 {
 }
 
@@ -1302,6 +1322,12 @@ bool nsCSPScriptSrcDirective::equals(CSPDirective aDirective) const
   if (aDirective == nsIContentSecurityPolicy::WORKER_SRC_DIRECTIVE) {
     return mRestrictWorkers;
   }
+  if (aDirective == nsIContentSecurityPolicy::SCRIPT_SRC_ELEM_DIRECTIVE) {
+    return mRestrictScriptElem;
+  }
+  if (aDirective == nsIContentSecurityPolicy::SCRIPT_SRC_ATTR_DIRECTIVE) {
+    return mRestrictScriptAttr;
+  }
   return (mDirective == aDirective);
 }
 

dom/security/nsCSPUtils.h

@@ -141,7 +141,9 @@ static const char* CSPStrDirectives[] = {
   "block-all-mixed-content",   // BLOCK_ALL_MIXED_CONTENT
   "require-sri-for",           // REQUIRE_SRI_FOR
   "sandbox",                   // SANDBOX_DIRECTIVE
-  "worker-src"                 // WORKER_SRC_DIRECTIVE
+  "worker-src",                // WORKER_SRC_DIRECTIVE
+  "script-src-elem",           // SCRIPT_SRC_ELEM_DIRECTIVE
+  "script-src-attr"            // SCRIPT_SRC_ATTR_DIRECTIVE
 };
 
 inline const char* CSP_CSPDirectiveToString(CSPDirective aDir)
@@ -537,13 +539,16 @@ class nsCSPScriptSrcDirective : public nsCSPDirective {
     explicit nsCSPScriptSrcDirective(CSPDirective aDirective);
     virtual ~nsCSPScriptSrcDirective();
 
-    void setRestrictWorkers()
+    void setRestrictWorkers()    { mRestrictWorkers = true; }
-      { mRestrictWorkers = true; }
+    void setRestrictScriptElem() { mRestrictScriptElem = true; }
+    void setRestrictScriptAttr() { mRestrictScriptAttr = true; }
 
     virtual bool equals(CSPDirective aDirective) const;
 
   private:
     bool mRestrictWorkers;
+    bool mRestrictScriptElem;
+    bool mRestrictScriptAttr;
 };
 
 /* =============== nsBlockAllMixedContentDirective === */

dom/security/test/unit/test_csp_reports.js

@@ -215,7 +215,7 @@ function run_test() {
     function(csp) {
       var uri = NetUtil
       // shouldLoad creates and sends out the report here.
-      csp.shouldLoad(Ci.nsIContentSecurityPolicy.SCRIPT_SRC_DIRECTIVE,
+      csp.shouldLoad(Ci.nsIContentSecurityPolicy.SCRIPT_SRC_ELEM_DIRECTIVE,
                      NetUtil.newURI(selfSpec + "#bar"),
                      null, null, null, null);
       });
@@ -224,7 +224,7 @@ function run_test() {
   makeTest(8, {"blocked-uri": "ftp://blocked.test"}, false,
     function(csp) {
       // shouldLoad creates and sends out the report here.
-      csp.shouldLoad(Ci.nsIContentSecurityPolicy.SCRIPT_SRC_DIRECTIVE,
+      csp.shouldLoad(Ci.nsIContentSecurityPolicy.SCRIPT_SRC_ELEM_DIRECTIVE,
                     NetUtil.newURI("ftp://blocked.test/profile.png"),
                     null, null, null, null);
     });

dom/webidl/CSPDictionaries.webidl

@@ -20,7 +20,7 @@ dictionary CSP {
   sequence<DOMString> connect-src;
   sequence<DOMString> report-uri;
   sequence<DOMString> frame-ancestors;
-  // sequence<DOMString> reflected-xss; // not supported in Firefox
+  // sequence<DOMString> reflected-xss; // not supported in UXP
   sequence<DOMString> base-uri;
   sequence<DOMString> form-action;
   sequence<DOMString> referrer;
@@ -31,6 +31,8 @@ dictionary CSP {
   sequence<DOMString> require-sri-for;
   sequence<DOMString> sandbox;
   sequence<DOMString> worker-src;
+  sequence<DOMString> script-src-elem;
+  sequence<DOMString> script-src-attr;
 };
 
 dictionary CSPPolicies {