Skip to content

Commit 58be4c8

Browse files
committed
Bug 1935434 Implement forgiving parsing for trusted-types CSP directive. r=smaug
Currently, we just discard the whole directive if an invalid token is found. With this patch, we instead ignore such a token. Also improves tests in should-trusted-type-policy-creation-be-blocked-by-csp-002.html so that we really check that the original trusted-types directive is preserved after serialization. See w3c/webappsec-csp#363 (comment) Differential Revision: https://phabricator.services.mozilla.com/D243358
1 parent 177c187 commit 58be4c8

File tree

6 files changed

+43
-24
lines changed

6 files changed

+43
-24
lines changed

dom/security/nsCSPParser.cpp

+3-7
Original file line numberDiff line numberDiff line change
@@ -990,14 +990,10 @@ void nsCSPParser::handleTrustedTypesDirective(nsCSPDirective* aDir) {
990990
new nsCSPTrustedTypesDirectivePolicyName(mCurToken));
991991
} else {
992992
AutoTArray<nsString, 1> token = {mCurToken};
993-
logWarningErrorToConsole(nsIScriptError::errorFlag,
993+
logWarningErrorToConsole(nsIScriptError::warningFlag,
994994
"invalidTrustedTypesExpression", token);
995-
996-
for (auto* trustedTypeExpression : trustedTypesExpressions) {
997-
delete trustedTypeExpression;
998-
}
999-
1000-
return;
995+
trustedTypesExpressions.AppendElement(
996+
new nsCSPTrustedTypesDirectiveInvalidToken(mCurToken));
1001997
}
1002998
}
1003999

dom/security/nsCSPUtils.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,24 @@ void nsCSPTrustedTypesDirectivePolicyName::toString(nsAString& aOutStr) const {
10851085
aOutStr.Append(mName);
10861086
}
10871087

1088+
/* =============== nsCSPTrustedTypesDirectiveInvalidToken =============== */
1089+
1090+
nsCSPTrustedTypesDirectiveInvalidToken::nsCSPTrustedTypesDirectiveInvalidToken(
1091+
const nsAString& aInvalidToken)
1092+
: mInvalidToken{aInvalidToken} {}
1093+
1094+
bool nsCSPTrustedTypesDirectiveInvalidToken::visit(
1095+
nsCSPSrcVisitor* aVisitor) const {
1096+
MOZ_ASSERT_UNREACHABLE(
1097+
"Should only be called for other overloads of this method.");
1098+
return false;
1099+
}
1100+
1101+
void nsCSPTrustedTypesDirectiveInvalidToken::toString(
1102+
nsAString& aOutStr) const {
1103+
aOutStr.Append(mInvalidToken);
1104+
}
1105+
10881106
/* ===== nsCSPDirective ====================== */
10891107

10901108
nsCSPDirective::nsCSPDirective(CSPDirective aDirective) {

dom/security/nsCSPUtils.h

+13
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,19 @@ class nsCSPTrustedTypesDirectivePolicyName : public nsCSPBaseSrc {
437437
const nsString mName;
438438
};
439439

440+
class nsCSPTrustedTypesDirectiveInvalidToken : public nsCSPBaseSrc {
441+
public:
442+
explicit nsCSPTrustedTypesDirectiveInvalidToken(
443+
const nsAString& aInvalidToken);
444+
virtual ~nsCSPTrustedTypesDirectiveInvalidToken() = default;
445+
446+
bool visit(nsCSPSrcVisitor* aVisitor) const override;
447+
void toString(nsAString& aOutStr) const override;
448+
449+
private:
450+
const nsString mInvalidToken;
451+
};
452+
440453
/* =============== nsCSPSrcVisitor ================== */
441454

442455
class nsCSPSrcVisitor {

dom/security/test/gtest/TestCSPParser.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,12 @@ TEST(CSPParser, Directives)
228228
"script-src http://example.com"},
229229
{ "require-trusted-types-for 'script'",
230230
"require-trusted-types-for 'script'" },
231+
{ "require-trusted-types-for 'script' invalid",
232+
"require-trusted-types-for 'script' 'invalid'"}, // bug 1956731
231233
{ "trusted-types somePolicyName", "trusted-types somePolicyName" },
232234
{ "trusted-types somePolicyName anotherPolicyName 1 - # = _ / @ . % *",
233235
"trusted-types somePolicyName anotherPolicyName 1 - # = _ / @ . % *" },
236+
{ "trusted-types $", "trusted-types 'invalid'" }, // bug 1935434
234237
// clang-format on
235238
};
236239

@@ -609,8 +612,8 @@ TEST(CSPParser, BadPolicies)
609612
{ "report-uri http://:foo", ""},
610613
{ "require-sri-for", ""},
611614
{ "require-sri-for style", ""},
612-
{ "trusted-types $", ""},
613-
{ "trusted-types 'report-sample'", "" },
615+
{ "require-trusted-types-for invalid" },
616+
{ "require-trusted-types-for 'invalid'" },
614617

615618
// clang-format on
616619
};

testing/web-platform/meta/trusted-types/should-trusted-type-policy-creation-be-blocked-by-csp-002.html.ini

-15
This file was deleted.

testing/web-platform/tests/trusted-types/should-trusted-type-policy-creation-be-blocked-by-csp-002.html

+4
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@
6363
// https://w3c.github.io/trusted-types/dist/spec/#should-block-create-policy
6464
assert_true(results[0].exception instanceof TypeError, "createPolicy() should throw a TypeError.");
6565
assert_equals(results[0].violatedPolicies.length, 1, "createPolicy() should trigger a violation report.");
66+
assert_equals(results[0].violatedPolicies[0].disposition, "enforce");
67+
assert_equals(results[0].violatedPolicies[0].policy, `trusted-types ${trustedTypePolicyName}`);
6668
}, `invalid tt-policy-name name "${trustedTypePolicyName}"`);
6769
});
6870

@@ -90,5 +92,7 @@
9092
assert_equals(results.length, 1);
9193
assert_true(results[0].exception instanceof TypeError);
9294
assert_equals(results[0].violatedPolicies.length, 1);
95+
assert_equals(results[0].violatedPolicies[0].disposition, "enforce");
96+
assert_equals(results[0].violatedPolicies[0].policy, `trusted-types _TTP_*`);
9397
}, `invalid directive "trusted-type _TTP" (no ascii whitespace)`);
9498
</script>

0 commit comments

Comments
 (0)