Commit 10f96d6f authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

Fix CSP reporting for unsafe eval in report-only.

The Content-Security-Policy-Report-Only header with script-src was not
checked for "eval". This patch fixed the issue.

---

This is mostly caused by the function:
~~~
void ScriptController::UpdateDocument() {
  window_proxy_manager_->MainWorldProxyMaybeUninitialized()->UpdateDocument();
  EnableEval();
}
~~~

Line 1 makes V8 to bypass checking CSP "eval" if there are no CSP or
TrustedType checks to make. So far so good!

Line 2 made V8 to bypass checking CSP "eval" unconditionally. Wrong!

---

The second problem was that V8 AllowCodeGenerationFromStrings was not
set to false in case of 'report-only' CSP. This patch fixed this and
went further. Blink now requires V8 to always call it back before any
'eval' call, no matter if there are policies or not. This is simpler.

---

Bug: 980127
Change-Id: Ia5b4887cc981d85a077649df8a54cfebace245ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947741
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarDaniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721432}
parent 78709537
......@@ -199,17 +199,14 @@ void LocalWindowProxy::Initialize() {
(world_->IsIsolatedWorld() &&
IsolatedWorldCSP::Get().HasContentSecurityPolicy(world_->GetWorldId()));
if (evaluate_csp_for_eval) {
// Using 'false' here means V8 will always call back blink for every 'eval'
// call being made. Blink executes CSP checks and returns whether or not
// V8 can proceed. The callback is
// V8Initializer::CodeGenerationCheckCallbackInMainThread().
context->AllowCodeGenerationFromStrings(false);
ContentSecurityPolicy* csp =
GetFrame()->GetDocument()->GetContentSecurityPolicyForWorld();
// CSP has two mechanisms for controlling eval, script-src and Trusted
// Types, and we need to check both.
// 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(
V8String(GetIsolate(), csp->EvalDisabledErrorMessage()));
}
......
......@@ -229,7 +229,6 @@ void ScriptController::ClearWindowProxy() {
void ScriptController::UpdateDocument() {
window_proxy_manager_->MainWorldProxyMaybeUninitialized()->UpdateDocument();
EnableEval();
}
void ScriptController::ExecuteJavaScriptURL(
......
This is a testharness.js-based test.
PASS Eval is allowed because the CSP is report-only
FAIL SPV event is still raised assert_unreached: SPV event has not been received Reached unreachable code
FAIL Violation report status OK. assert_equals: No such report. expected "" but got "false"
Harness: the test ran to completion.
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