Skip to content

Commit

Permalink
Backed out 6 changesets (bug 1740263) for causing bp-hybrid bustages …
Browse files Browse the repository at this point in the history
…on nsScriptSecurityManager. CLOSED TREE

Backed out changeset 2f5ec6ad0f81 (bug 1740263)
Backed out changeset a1e7766cdb94 (bug 1740263)
Backed out changeset 3978ccb95455 (bug 1740263)
Backed out changeset e34ba774b3f8 (bug 1740263)
Backed out changeset 8365b10be28e (bug 1740263)
Backed out changeset d923462c9cd0 (bug 1740263)
  • Loading branch information
Iulian Moraru committed May 19, 2022
1 parent 283a88b commit 4d59317
Show file tree
Hide file tree
Showing 47 changed files with 174 additions and 585 deletions.
10 changes: 2 additions & 8 deletions caps/nsIAddonPolicyService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ interface nsIAddonPolicyService : nsISupports
*/
readonly attribute AString defaultCSP;

/**
* Same as above, but used for extensions using manifest v3.
*/
readonly attribute AString defaultCSPV3;

/**
* Returns the base content security policy which applies to all extension resources.
*/
Expand Down Expand Up @@ -84,14 +79,13 @@ interface nsIAddonContentPolicy : nsISupports
/* options to pass to validateAddonCSP
*
* Manifest V2 uses CSP_ALLOW_ANY.
* In Manifest V3, extension_pages would use CSP_ALLOW_LOCALHOST|CSP_ALLOW_WASM
* and sandbox would use CSP_ALLOW_EVAL.
* In Manifest V3, extension_pages would use CSP_ALLOW_LOCALHOST and
* sandbox would use CSP_ALLOW_EVAL.
*/
const unsigned long CSP_ALLOW_ANY = 0xFFFF;
const unsigned long CSP_ALLOW_LOCALHOST = (1<<0);
const unsigned long CSP_ALLOW_EVAL = (1<<1);
const unsigned long CSP_ALLOW_REMOTE = (1<<2);
const unsigned long CSP_ALLOW_WASM = (1<<3);

/**
* Checks a custom content security policy string, to ensure that it meets
Expand Down
52 changes: 20 additions & 32 deletions caps/nsScriptSecurityManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ NS_IMPL_ISUPPORTS(nsScriptSecurityManager, nsIScriptSecurityManager)
///////////////// Security Checks /////////////////

bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
JSContext* cx, JS::RuntimeCode aKind, JS::Handle<JSString*> aCode) {
JSContext* cx, JS::HandleString aCode) {
MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext());

// Get the window, if any, corresponding to the current global
Expand Down Expand Up @@ -484,37 +484,30 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(

bool evalOK = true;
bool reportViolation = false;
nsresult rv = csp->GetAllowsEval(&reportViolation, &evalOK);

// A little convoluted. We want the scriptSample for a) reporting a violation
// or b) passing it to AssertEvalNotUsingSystemPrincipal or c) we're in the
// parent process. So do the work to get it if either of those cases is true.
nsAutoJSString scriptSample;
if (aKind == JS::RuntimeCode::JS) {
nsresult rv = csp->GetAllowsEval(&reportViolation, &evalOK);

// A little convoluted. We want the scriptSample for a) reporting a
// violation or b) passing it to AssertEvalNotUsingSystemPrincipal or c)
// we're in the parent process. So do the work to get it if either of those
// cases is true.
if (reportViolation || subjectPrincipal->IsSystemPrincipal() ||
XRE_IsE10sParentProcess()) {
if (NS_WARN_IF(!scriptSample.init(cx, aCode))) {
JS_ClearPendingException(cx);
return false;
}
if (reportViolation || subjectPrincipal->IsSystemPrincipal() ||
XRE_IsE10sParentProcess()) {
if (NS_WARN_IF(!scriptSample.init(cx, aCode))) {
JS_ClearPendingException(cx);
return false;
}
}

#if !defined(ANDROID)
if (!nsContentSecurityUtils::IsEvalAllowed(
cx, subjectPrincipal->IsSystemPrincipal(), scriptSample)) {
return false;
}
if (!nsContentSecurityUtils::IsEvalAllowed(
cx, subjectPrincipal->IsSystemPrincipal(), scriptSample)) {
return false;
}
#endif

if (NS_FAILED(rv)) {
NS_WARNING("CSP: failed to get allowsEval");
return true; // fail open to not break sites.
}
} else {
if (NS_FAILED(csp->GetAllowsWasmEval(&reportViolation, &evalOK))) {
return false;
}
if (NS_FAILED(rv)) {
NS_WARNING("CSP: failed to get allowsEval");
return true; // fail open to not break sites.
}

if (reportViolation) {
Expand All @@ -529,12 +522,7 @@ bool nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(
} else {
MOZ_ASSERT(!JS_IsExceptionPending(cx));
}

uint16_t violationType =
aKind == JS::RuntimeCode::JS
? nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL
: nsIContentSecurityPolicy::VIOLATION_TYPE_WASM_EVAL;
csp->LogViolationDetails(violationType,
csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL,
nullptr, // triggering element
cspEventListener, fileName, scriptSample, lineNum,
columnNum, u""_ns, u""_ns);
Expand Down
3 changes: 1 addition & 2 deletions caps/nsScriptSecurityManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ class nsScriptSecurityManager final : public nsIScriptSecurityManager {

// Decides, based on CSP, whether or not eval() and stuff can be executed.
static bool ContentSecurityPolicyPermitsJSAction(JSContext* cx,
JS::RuntimeCode kind,
JS::Handle<JSString*> aCode);
JS::HandleString aCode);

static bool JSPrincipalsSubsume(JSPrincipals* first, JSPrincipals* second);

Expand Down
17 changes: 1 addition & 16 deletions dom/interfaces/security/nsIContentSecurityPolicy.idl
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ interface nsIContentSecurityPolicy : nsISerializable
in boolean aEnforceWhitelist);

/**
* Whether this policy allows eval and eval-like functions
* whether this policy allows eval and eval-like functions
* such as setTimeout("code string", time).
* @param shouldReportViolations
* Whether or not the use of eval should be reported.
Expand All @@ -178,20 +178,6 @@ interface nsIContentSecurityPolicy : nsISerializable
*/
boolean getAllowsEval(out boolean shouldReportViolations);

/**
* Whether this policy allows the evaluation (and compilation) of
* WASM code from functions like `WebAssembly.compile`.
* @param shouldReportViolations
* Whether or not the use of WASM evaluation should be reported.
* This function returns "true" when violating report-only policies, but
* when any policy (report-only or otherwise) is violated,
* shouldReportViolations is true as well.
* @return
* Whether or not the effects of the WASM evaluation should be allowed
* (block the call if false).
*/
boolean getAllowsWasmEval(out boolean shouldReportViolations);

/**
* Delegate method called by the service when the protected document is loaded.
* Returns the union of all the sandbox flags contained in CSP policies. This is the most
Expand Down Expand Up @@ -249,7 +235,6 @@ interface nsIContentSecurityPolicy : nsISerializable
const unsigned short VIOLATION_TYPE_HASH_STYLE = 7;
const unsigned short VIOLATION_TYPE_REQUIRE_SRI_FOR_STYLE = 8;
const unsigned short VIOLATION_TYPE_REQUIRE_SRI_FOR_SCRIPT = 9;
const unsigned short VIOLATION_TYPE_WASM_EVAL = 10;

/**
* Called after the CSP object is created to fill in appropriate request
Expand Down
1 change: 0 additions & 1 deletion dom/security/CSPEvalChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ nsresult CheckInternal(nsIContentSecurityPolicy* aCSP,
// The value is set at any "return", but better to have a default value here.
*aAllowed = false;

// This is the non-CSP check for gating eval() use in the SystemPrincipal
#if !defined(ANDROID)
JSContext* cx = nsContentUtils::GetCurrentJSContext();
if (!nsContentSecurityUtils::IsEvalAllowed(
Expand Down
45 changes: 0 additions & 45 deletions dom/security/nsCSPContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "nsStringStream.h"
#include "mozilla/Logging.h"
#include "mozilla/Preferences.h"
#include "mozilla/StaticPrefs_security.h"
#include "mozilla/dom/CSPReportBinding.h"
#include "mozilla/dom/CSPDictionariesBinding.h"
#include "mozilla/ipc/PBackgroundSharedTypes.h"
Expand Down Expand Up @@ -118,10 +117,6 @@ static void BlockedContentSourceToString(
case nsCSPContext::BlockedContentSource::eSelf:
aString.AssignLiteral("self");
break;

case nsCSPContext::BlockedContentSource::eWasmEval:
aString.AssignLiteral("wasm-eval");
break;
}
}

Expand Down Expand Up @@ -476,36 +471,6 @@ nsCSPContext::GetAllowsEval(bool* outShouldReportViolation,
return NS_OK;
}

NS_IMETHODIMP
nsCSPContext::GetAllowsWasmEval(bool* outShouldReportViolation,
bool* outAllowsWasmEval) {
EnsureIPCPoliciesRead();
*outShouldReportViolation = false;
*outAllowsWasmEval = true;

if (!StaticPrefs::security_csp_wasm_unsafe_eval_enabled()) {
// Allow and don't report when wasm-unsafe-eval isn't supported.
return NS_OK;
}

for (uint32_t i = 0; i < mPolicies.Length(); i++) {
// Either 'unsafe-eval' or 'wasm-unsafe-eval' can allow this
if (!mPolicies[i]->allows(SCRIPT_SRC_DIRECTIVE, CSP_WASM_UNSAFE_EVAL,
u""_ns, false) &&
!mPolicies[i]->allows(SCRIPT_SRC_DIRECTIVE, CSP_UNSAFE_EVAL, u""_ns,
false)) {
// policy is violated: must report the violation and allow the inline
// script if the policy is report-only.
*outShouldReportViolation = true;
if (!mPolicies[i]->getReportOnlyFlag()) {
*outAllowsWasmEval = false;
}
}
}

return NS_OK;
}

// Helper function to report inline violations
void nsCSPContext::reportInlineViolation(
CSPDirective aDirective, Element* aTriggeringElement,
Expand Down Expand Up @@ -754,11 +719,6 @@ nsCSPContext::GetAllowsNavigateTo(nsIURI* aURI, bool aIsFormSubmission,
bool reportSample = false; \
mPolicies[p]->getDirectiveStringAndReportSampleForContentType( \
directive##_SRC_DIRECTIVE, violatedDirective, &reportSample); \
if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL || \
aViolationType == \
nsIContentSecurityPolicy::VIOLATION_TYPE_WASM_EVAL) { \
violatedDirective = u"script-src"_ns; \
} \
AsyncReportViolation(aTriggeringElement, aCSPEventListener, nullptr, \
blockedContentSource, nullptr, violatedDirective, \
p, NS_LITERAL_STRING_FROM_CSTRING(observerTopic), \
Expand Down Expand Up @@ -805,9 +765,6 @@ nsCSPContext::LogViolationDetails(
BlockedContentSource blockedContentSource = BlockedContentSource::eUnknown;
if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL) {
blockedContentSource = BlockedContentSource::eEval;
} else if (aViolationType ==
nsIContentSecurityPolicy::VIOLATION_TYPE_WASM_EVAL) {
blockedContentSource = BlockedContentSource::eWasmEval;
} else if (aViolationType ==
nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT ||
aViolationType ==
Expand All @@ -834,8 +791,6 @@ nsCSPContext::LogViolationDetails(
SCRIPT_HASH_VIOLATION_OBSERVER_TOPIC);
CASE_CHECK_AND_REPORT(HASH_STYLE, STYLE, aContent, CSP_UNSAFE_INLINE,
STYLE_HASH_VIOLATION_OBSERVER_TOPIC);
CASE_CHECK_AND_REPORT(WASM_EVAL, SCRIPT, u""_ns, CSP_WASM_UNSAFE_EVAL,
WASM_EVAL_VIOLATION_OBSERVER_TOPIC);

default:
NS_ASSERTION(false, "LogViolationDetails with invalid type");
Expand Down
1 change: 0 additions & 1 deletion dom/security/nsCSPContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ class nsCSPContext : public nsIContentSecurityPolicy {
eInline,
eEval,
eSelf,
eWasmEval,
};

nsresult AsyncReportViolation(
Expand Down
8 changes: 1 addition & 7 deletions dom/security/nsCSPParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,7 @@ nsCSPHostSrc* nsCSPParser::host() {
return new nsCSPHostSrc(mCurValue);
}

// keyword-source = "'self'" / "'unsafe-inline'" / "'unsafe-eval'" /
// "'wasm-unsafe-eval'"
// keyword-source = "'self'" / "'unsafe-inline'" / "'unsafe-eval'"
nsCSPBaseSrc* nsCSPParser::keywordSource() {
CSPPARSERLOG(("nsCSPParser::keywordSource, mCurToken: %s, mCurValue: %s",
NS_ConvertUTF16toUTF8(mCurToken).get(),
Expand Down Expand Up @@ -441,11 +440,6 @@ nsCSPBaseSrc* nsCSPParser::keywordSource() {
return new nsCSPKeywordSrc(CSP_UTF16KeywordToEnum(mCurToken));
}

if (StaticPrefs::security_csp_wasm_unsafe_eval_enabled() &&
CSP_IsKeyword(mCurToken, CSP_WASM_UNSAFE_EVAL)) {
return new nsCSPKeywordSrc(CSP_UTF16KeywordToEnum(mCurToken));
}

if (CSP_IsKeyword(mCurToken, CSP_UNSAFE_ALLOW_REDIRECTS)) {
if (!CSP_IsDirective(mCurDir[0],
nsIContentSecurityPolicy::NAVIGATE_TO_DIRECTIVE)) {
Expand Down
7 changes: 3 additions & 4 deletions dom/security/nsCSPUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -880,12 +880,11 @@ bool nsCSPKeywordSrc::allows(enum CSPKeyword aKeyword,
}
// either the keyword allows the load or the policy contains 'strict-dynamic',
// in which case we have to make sure the script is not parser created before
// allowing the load and also eval & wasm-eval should be blocked even if
// 'strict-dynamic' is present. Should be allowed only if 'unsafe-eval' is
// present.
// allowing the load and also eval should be blocked even if 'strict-dynamic'
// is present. Should be allowed only if 'unsafe-eval' is present.
return ((mKeyword == aKeyword) ||
((mKeyword == CSP_STRICT_DYNAMIC) && !aParserCreated &&
aKeyword != CSP_UNSAFE_EVAL && aKeyword != CSP_WASM_UNSAFE_EVAL));
aKeyword != CSP_UNSAFE_EVAL));
}

bool nsCSPKeywordSrc::visit(nsCSPSrcVisitor* aVisitor) const {
Expand Down
22 changes: 9 additions & 13 deletions dom/security/nsCSPUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ void CSP_LogMessage(const nsAString& aMessage, const nsAString& aSourceName,
"violated base restriction: Inline Scripts will not execute"
#define EVAL_VIOLATION_OBSERVER_TOPIC \
"violated base restriction: Code will not be created from strings"
#define WASM_EVAL_VIOLATION_OBSERVER_TOPIC \
"violated base restriction: WebAssembly code will not be created from " \
"dynamically"
#define SCRIPT_NONCE_VIOLATION_OBSERVER_TOPIC "Inline Script had invalid nonce"
#define STYLE_NONCE_VIOLATION_OBSERVER_TOPIC "Inline Style had invalid nonce"
#define SCRIPT_HASH_VIOLATION_OBSERVER_TOPIC "Inline Script had invalid hash"
Expand Down Expand Up @@ -108,16 +105,15 @@ inline CSPDirective CSP_StringToCSPDirective(const nsAString& aDir) {
return nsIContentSecurityPolicy::NO_DIRECTIVE;
}

#define FOR_EACH_CSP_KEYWORD(MACRO) \
MACRO(CSP_SELF, "'self'") \
MACRO(CSP_UNSAFE_INLINE, "'unsafe-inline'") \
MACRO(CSP_UNSAFE_EVAL, "'unsafe-eval'") \
MACRO(CSP_NONE, "'none'") \
MACRO(CSP_NONCE, "'nonce-") \
MACRO(CSP_REPORT_SAMPLE, "'report-sample'") \
MACRO(CSP_STRICT_DYNAMIC, "'strict-dynamic'") \
MACRO(CSP_UNSAFE_ALLOW_REDIRECTS, "'unsafe-allow-redirects'") \
MACRO(CSP_WASM_UNSAFE_EVAL, "'wasm-unsafe-eval'")
#define FOR_EACH_CSP_KEYWORD(MACRO) \
MACRO(CSP_SELF, "'self'") \
MACRO(CSP_UNSAFE_INLINE, "'unsafe-inline'") \
MACRO(CSP_UNSAFE_EVAL, "'unsafe-eval'") \
MACRO(CSP_NONE, "'none'") \
MACRO(CSP_NONCE, "'nonce-") \
MACRO(CSP_REPORT_SAMPLE, "'report-sample'") \
MACRO(CSP_STRICT_DYNAMIC, "'strict-dynamic'") \
MACRO(CSP_UNSAFE_ALLOW_REDIRECTS, "'unsafe-allow-redirects'")

enum CSPKeyword {
#define KEYWORD_ENUM(id_, string_) id_,
Expand Down
7 changes: 0 additions & 7 deletions dom/security/test/gtest/TestCSPParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,9 @@ nsresult runTestSuite(const PolicyTest* aPolicies, uint32_t aPolicyCount,
nsresult rv;
nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
bool navigateTo = false;
bool wasmUnsafeEval = false;
if (prefs) {
prefs->GetBoolPref("security.csp.enableNavigateTo", &navigateTo);
prefs->SetBoolPref("security.csp.enableNavigateTo", true);
prefs->GetBoolPref("security.csp.wasm-unsafe-eval.enabled",
&wasmUnsafeEval);
prefs->SetBoolPref("security.csp.wasm-unsafe-eval.enabled", true);
}

for (uint32_t i = 0; i < aPolicyCount; i++) {
Expand All @@ -167,7 +163,6 @@ nsresult runTestSuite(const PolicyTest* aPolicies, uint32_t aPolicyCount,

if (prefs) {
prefs->SetBoolPref("security.csp.enableNavigateTo", navigateTo);
prefs->SetBoolPref("security.csp.wasm-unsafe-eval.enabled", wasmUnsafeEval);
}

return NS_OK;
Expand Down Expand Up @@ -250,8 +245,6 @@ TEST(CSPParser, Keywords)
"script-src 'unsafe-inline' 'unsafe-eval'" },
{ "script-src 'none'",
"script-src 'none'" },
{ "script-src 'wasm-unsafe-eval'",
"script-src 'wasm-unsafe-eval'" },
{ "img-src 'none'; script-src 'unsafe-eval' 'unsafe-inline'; default-src 'self'",
"img-src 'none'; script-src 'unsafe-eval' 'unsafe-inline'; default-src 'self'" },
// clang-format on
Expand Down
4 changes: 1 addition & 3 deletions dom/serviceworkers/ServiceWorkerPrivate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1759,9 +1759,7 @@ nsresult ServiceWorkerPrivate::SpawnWorkerIfNeeded(WakeUpReason aWhy,
// Default CSP permissions for now. These will be overrided if necessary
// based on the script CSP headers during load in ScriptLoader.
info.mEvalAllowed = true;
info.mReportEvalCSPViolations = false;
info.mWasmEvalAllowed = true;
info.mReportWasmEvalCSPViolations = false;
info.mReportCSPViolations = false;

WorkerPrivate::OverrideLoadInfoLoadGroup(info, info.mPrincipal);

Expand Down
Loading

0 comments on commit 4d59317

Please sign in to comment.