Skip to content

GitLab

T
tangled
Commit 5e337a86 authored by Daniel Vogelheim's avatar Daniel Vogelheim Committed by Commit Bot
Browse files
Options

[Trusted Types] De-flake trusted-types-eval-reporting test. 

The WPT test trusted-types-eval-reporting was flaky, usually failing on the
first run, but succeeding on the second. This was a real (and potentially
serious) bug, where the CSP logic would rely on the V8 eval callback being
called, but other parts would blanket-allow eval (and hence suppress the
callback) because they didn't check for 'trusted types' condition.

Also, minor cleanups for the test.

Bug: 739170
Change-Id: I00699b115f0be474c2b6d231dcc8b884868248b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807218
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698883}
parent 9611515e
Show whitespace changes
Inline Side-by-side
Showing with 21 additions and 20 deletions
third_party/blink/renderer/bindings/core/v8/local_window_proxy.cc
View file @ 5e337a86
...@@ -182,9 +182,15 @@ void LocalWindowProxy::Initialize() { ...@@ -182,9 +182,15 @@ void LocalWindowProxy::Initialize() {
if (evaluate_csp_for_eval) { if (evaluate_csp_for_eval) {
ContentSecurityPolicy* csp = ContentSecurityPolicy* csp =
GetFrame()->GetDocument()->GetContentSecurityPolicyForWorld(); GetFrame()->GetDocument()->GetContentSecurityPolicyForWorld();
context->AllowCodeGenerationFromStrings(csp->AllowEval( // CSP has two mechanisms for controlling eval, script-src and Trusted
SecurityViolationReportingPolicy::kSuppressReporting, // Types, and we need to check both.
ContentSecurityPolicy::kWillNotThrowException, g_empty_string)); // TODO(vogelheim): Provide a simple(e) API for this use case.
bool allow_code_generation =
csp->AllowEval(SecurityViolationReportingPolicy::kSuppressReporting,
ContentSecurityPolicy::kWillNotThrowException,
g_empty_string) &&
!csp->IsRequireTrustedTypes();
context->AllowCodeGenerationFromStrings(allow_code_generation);
context->SetErrorMessageForCodeGenerationFromStrings( context->SetErrorMessageForCodeGenerationFromStrings(
V8String(GetIsolate(), csp->EvalDisabledErrorMessage())); V8String(GetIsolate(), csp->EvalDisabledErrorMessage()));
} }
......
third_party/blink/web_tests/external/wpt/trusted-types/trusted-types-eval-reporting.tentative.https.html
View file @ 5e337a86
<!DOCTYPE html> <!DOCTYPE html>
<head> <head>
<script nonce="123" src="/resources/testharness.js"></script> <script nonce="123" src="/resources/testharness.js"></script>
<script nonce="123"src="/resources/testharnessreport.js"></script> <script nonce="123" src="/resources/testharnessreport.js"></script>
<script nonce="123"src="/content-security-policy/support/testharness-helper.js"></script>
</head> </head>
<body> <body>
<script nonce="123"> <script nonce="123">
...@@ -23,15 +22,10 @@ ...@@ -23,15 +22,10 @@
// Return function that returns a promise that resolves on the given // Return function that returns a promise that resolves on the given
// violation report. // violation report.
//
// filter_arg - iff function, call it with the event object.
// Else, string-ify and compare against event.originalPolicy.
function promise_violation(filter_arg) { function promise_violation(filter_arg) {
return _ => new Promise((resolve, reject) => { return _ => new Promise((resolve, reject) => {
function handler(e) { function handler(e) {
let matches = (filter_arg instanceof Function) let matches = e.originalPolicy.includes(filter_arg);
? filter_arg(e)
: (e.originalPolicy.includes(filter_arg));
if (matches) { if (matches) {
document.removeEventListener("securitypolicyviolation", handler); document.removeEventListener("securitypolicyviolation", handler);
e.stopPropagation(); e.stopPropagation();
...@@ -56,7 +50,6 @@ ...@@ -56,7 +50,6 @@
createURL: id, createURL: id,
createScript: id, createScript: id,
}; };
const scriptyPolicy = TrustedTypes.createPolicy('allowEval', a_policy); const scriptyPolicy = TrustedTypes.createPolicy('allowEval', a_policy);
// Provoke/wait for a CSP violation, in order to be sure that all previous // Provoke/wait for a CSP violation, in order to be sure that all previous
...@@ -72,33 +65,35 @@ ...@@ -72,33 +65,35 @@
}); });
} }
window.script_run_beacon = 'never_overwritten';
promise_test(t => { promise_test(t => {
let beacon = 'never_overwritten';
let p = Promise.resolve() let p = Promise.resolve()
.then(promise_violation("trusted-types *")) .then(promise_violation("trusted-types *"))
.then(promise_flush()); .then(promise_flush());
expect_throws(_ => eval('script_run_beacon="should not run"')); assert_throws(new EvalError(),
assert_equals(script_run_beacon, 'never_overwritten'); _ => eval('beacon="should not run"'));
assert_equals(beacon, 'never_overwritten');
flush(); flush();
return p; return p;
}, "Trusted Type violation report: evaluating a string."); }, "Trusted Type violation report: evaluating a string.");
promise_test(t => { promise_test(t => {
let beacon = 'never_overwritten2';
let p = promise_flush()(); let p = promise_flush()();
eval(scriptyPolicy.createScript('script_run_beacon="i ran"')); eval(scriptyPolicy.createScript('beacon="i ran"'));
assert_equals(beacon, 'i ran');
flush(); flush();
assert_equals(script_run_beacon, 'i ran');
return p; return p;
}, "Trusted Type violation report: evaluating a Trusted Script."); }, "Trusted Type violation report: evaluating a Trusted Script.");
promise_test(t => { promise_test(t => {
let beacon = 'never_overwritten';
TrustedTypes.createPolicy('default', { TrustedTypes.createPolicy('default', {
createScript: s => s.replace('payload', 'default policy'), createScript: s => s.replace('payload', 'default policy'),
}, true); }, true);
let p = promise_flush()(); let p = promise_flush()();
eval('script_run_beacon="payload"'); eval('beacon="payload"');
assert_equals(script_run_beacon, 'default policy'); assert_equals(beacon, 'default policy');
flush(); flush();
return p; return p;
}, "Trusted Type violation report: default policy transforms the script before CSP checks runs."); }, "Trusted Type violation report: default policy transforms the script before CSP checks runs.");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment