diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 9a3b02f7f4..7db30be4e0 100644 --- a/dom/events/EventListenerManager.cpp +++ b/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, diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl index f01bb69add..f374226411 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/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 diff --git a/dom/jsurl/nsJSProtocolHandler.cpp b/dom/jsurl/nsJSProtocolHandler.cpp index aa7a3c3259..d16bafdc23 100644 --- a/dom/jsurl/nsJSProtocolHandler.cpp +++ b/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 diff --git a/dom/locales/en-US/chrome/security/csp.properties b/dom/locales/en-US/chrome/security/csp.properties index da38227403..28ad5afd91 100644 --- a/dom/locales/en-US/chrome/security/csp.properties +++ b/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 -ignoringSrcWithinScriptStyleSrc = Ignoring “%1$S” within script-src or style-src: nonce-source or hash-source specified -# LOCALIZATION NOTE (ignoringSrcForStrictDynamic): -# %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 +# %2$S is the directive src (e.g. "script-src-elem") +# 'strict-dynamic' should not be localized +ignoringScriptSrcForStrictDynamic = Ignoring “%1$S” within %2$S: ‘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 diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 3fc8027660..636816863d 100644 --- a/dom/script/ScriptLoader.cpp +++ b/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(), diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index da0a942d01..0569ddcb57 100644 --- a/dom/security/nsCSPContext.cpp +++ b/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) { - MOZ_ASSERT(false, "can only allow inline for script or style"); + if (aDirective != SCRIPT_SRC_ELEM_DIRECTIVE && + 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; } diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp index 1ec8194513..ddc45e2a8a 100644 --- a/dom/security/nsCSPParser.cpp +++ b/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), - "strict-dynamic only allowed within script-src"); + MOZ_ASSERT( + 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() }; - logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringSrcForStrictDynamic", + const char16_t* params[] = { srcStr.get(), mCurDir[0].get() }; + 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 - const char16_t* params[] = { u"'unsafe-inline'" }; - logWarningErrorToConsole(nsIScriptError::warningFlag, "ignoringSrcWithinScriptStyleSrc", + // log to the console that unsafe-inline will be ignored. + const char16_t* params[] = { u"'unsafe-inline'", mCurDir[0].get() }; + 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 - // not restrict frames; if not, than child-src needs to restrict frames. + // if frame-src is specified explicitly for that policy, then child-src should + // 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 - // not restrict workers; if not, than child-src needs to restrict workers. + // if worker-src is specified explicitly for that policy, then child-src should + // 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; } diff --git a/dom/security/nsCSPParser.h b/dom/security/nsCSPParser.h index 03ef2bb41e..1ba4179809 100644 --- a/dom/security/nsCSPParser.h +++ b/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 mStrictDynamic; // false, if 'strict-dynamic' is not defined + bool mHasHashOrNonce; // false, if no hash or nonce is 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; diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index aa0f52c4f8..93b0d4733c 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/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); } diff --git a/dom/security/nsCSPUtils.h b/dom/security/nsCSPUtils.h index 51a9a4d7b3..a2b2511e7b 100644 --- a/dom/security/nsCSPUtils.h +++ b/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() - { mRestrictWorkers = true; } + void setRestrictWorkers() { mRestrictWorkers = true; } + void setRestrictScriptElem() { mRestrictScriptElem = true; } + void setRestrictScriptAttr() { mRestrictScriptAttr = true; } virtual bool equals(CSPDirective aDirective) const; private: bool mRestrictWorkers; + bool mRestrictScriptElem; + bool mRestrictScriptAttr; }; /* =============== nsBlockAllMixedContentDirective === */ diff --git a/dom/security/test/unit/test_csp_reports.js b/dom/security/test/unit/test_csp_reports.js index d5a445750b..54dc470b4a 100644 --- a/dom/security/test/unit/test_csp_reports.js +++ b/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); }); diff --git a/dom/webidl/CSPDictionaries.webidl b/dom/webidl/CSPDictionaries.webidl index f8de1c9ad8..0c024fe61f 100644 --- a/dom/webidl/CSPDictionaries.webidl +++ b/dom/webidl/CSPDictionaries.webidl @@ -20,7 +20,7 @@ dictionary CSP { sequence connect-src; sequence report-uri; sequence frame-ancestors; - // sequence reflected-xss; // not supported in Firefox + // sequence reflected-xss; // not supported in UXP sequence base-uri; sequence form-action; sequence referrer; @@ -31,6 +31,8 @@ dictionary CSP { sequence require-sri-for; sequence sandbox; sequence worker-src; + sequence script-src-elem; + sequence script-src-attr; }; dictionary CSPPolicies {