Skip to content

Commit

Permalink
[CSP] Added new policy violation source: wasm-eval
Browse files Browse the repository at this point in the history
This extends the suite of policy violation sources to include
a WebAssembly specific source: wasm-eval.

This has also been reflected in the PR
(w3c/webappsec-csp#293 (review))
against the CSP spec.

Added test for proper security violation event of the right form.

Bug: 948834
Change-Id: I0b76fd725136b7ddda92e629f147f5ba77c50ffb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3197842
Commit-Queue: Francis McCabe <[email protected]>
Reviewed-by: Arthur Sonzogni <[email protected]>
Reviewed-by: Mike West <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Andrey Kosyakov <[email protected]>
Reviewed-by: David Tseng <[email protected]>
Cr-Commit-Position: refs/heads/main@{#931206}
NOKEYCHECK=True
GitOrigin-RevId: d476b181f81d3ab5926ae76cd051bdc989a692a8
  • Loading branch information
fgmccabe authored and copybara-github committed Oct 13, 2021
1 parent 7a05b72 commit 15ea6f7
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 2 deletions.
1 change: 1 addition & 0 deletions blink/public/devtools_protocol/browser_protocol.pdl
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ experimental domain Audits
kURLViolation
kTrustedTypesSinkViolation
kTrustedTypesPolicyViolation
kWasmEvalViolation

type SourceCodeLocation extends object
properties
Expand Down
1 change: 1 addition & 0 deletions blink/public/mojom/devtools/inspector_issue.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ enum ContentSecurityPolicyViolationType {
kURLViolation,
kTrustedTypesSinkViolation,
kTrustedTypesPolicyViolation,
kWasmEvalViolation,
};

struct ContentSecurityPolicyIssueDetails {
Expand Down
6 changes: 6 additions & 0 deletions blink/renderer/core/frame/csp/content_security_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,9 @@ std::unique_ptr<SourceLocation> GatherSecurityPolicyViolationEventData(
case ContentSecurityPolicyViolationType::kEvalViolation:
init->setBlockedURI("eval");
break;
case ContentSecurityPolicyViolationType::kWasmEvalViolation:
init->setBlockedURI("wasm-eval");
break;
case ContentSecurityPolicyViolationType::kURLViolation:
// We pass RedirectStatus::kNoRedirect so that StripURLForUseInReport
// does not strip path and query from the URL. This is safe since
Expand Down Expand Up @@ -1162,6 +1165,9 @@ ContentSecurityPolicy::BuildCSPViolationType(
switch (violation_type) {
case blink::ContentSecurityPolicyViolationType::kEvalViolation:
return mojom::blink::ContentSecurityPolicyViolationType::kEvalViolation;
case blink::ContentSecurityPolicyViolationType::kWasmEvalViolation:
return mojom::blink::ContentSecurityPolicyViolationType::
kWasmEvalViolation;
case blink::ContentSecurityPolicyViolationType::kInlineViolation:
return mojom::blink::ContentSecurityPolicyViolationType::kInlineViolation;
case blink::ContentSecurityPolicyViolationType::
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ enum ContentSecurityPolicyViolationType {
kEvalViolation,
kURLViolation,
kTrustedTypesSinkViolation,
kTrustedTypesPolicyViolation
kTrustedTypesPolicyViolation,
kWasmEvalViolation
};
} // namespace blink

Expand Down
31 changes: 30 additions & 1 deletion blink/renderer/core/frame/csp/csp_directive_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,35 @@ void ReportEvalViolation(
RedirectStatus::kNoRedirect, nullptr, content);
}

void ReportWasmEvalViolation(
const network::mojom::blink::ContentSecurityPolicy& csp,
ContentSecurityPolicy* policy,
const String& directive_text,
CSPDirectiveName effective_type,
const String& message,
const KURL& blocked_url,
const ContentSecurityPolicy::ExceptionStatus exception_status,
const String& content) {
String report_message =
CSPDirectiveListIsReportOnly(csp) ? "[Report Only] " + message : message;
// Print a console message if it won't be redundant with a JavaScript
// exception that the caller will throw. Exceptions will never get thrown in
// report-only mode because the caller won't see a violation.
if (CSPDirectiveListIsReportOnly(csp) ||
exception_status == ContentSecurityPolicy::kWillNotThrowException) {
auto* console_message = MakeGarbageCollected<ConsoleMessage>(
mojom::blink::ConsoleMessageSource::kSecurity,
mojom::blink::ConsoleMessageLevel::kError, report_message);
policy->LogToConsole(console_message);
}
policy->ReportViolation(
directive_text, effective_type, message, blocked_url,
csp.report_endpoints, csp.use_reporting_api, csp.header->header_value,
csp.header->type, ContentSecurityPolicyViolationType::kWasmEvalViolation,
std::unique_ptr<SourceLocation>(), nullptr, RedirectStatus::kNoRedirect,
nullptr, content);
}

bool CheckEval(const network::mojom::blink::CSPSourceList* directive) {
return !directive || directive->allow_eval;
}
Expand Down Expand Up @@ -400,7 +429,7 @@ bool CheckWasmEvalAndReportViolation(

String raw_directive =
GetRawDirectiveForMessage(csp.raw_directives, directive.type);
ReportEvalViolation(
ReportWasmEvalViolation(
csp, policy, raw_directive, CSPDirectiveName::ScriptSrc,
console_message + "\"" + raw_directive + "\"." + suffix + "\n", KURL(),
exception_status,
Expand Down
3 changes: 3 additions & 0 deletions blink/renderer/core/inspector/inspector_audits_issue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ protocol::Audits::ContentSecurityPolicyViolationType CSPViolationTypeToProtocol(
case ContentSecurityPolicyViolationType::kEvalViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KEvalViolation;
case ContentSecurityPolicyViolationType::kWasmEvalViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KWasmEvalViolation;
case ContentSecurityPolicyViolationType::kInlineViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KInlineViolation;
Expand Down
4 changes: 4 additions & 0 deletions blink/renderer/core/inspector/inspector_issue_conversion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,10 @@ protocol::String BuildViolationType(
kEvalViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KEvalViolation;
case blink::mojom::blink::ContentSecurityPolicyViolationType::
kWasmEvalViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KWasmEvalViolation;
case blink::mojom::blink::ContentSecurityPolicyViolationType::kURLViolation:
return protocol::Audits::ContentSecurityPolicyViolationTypeEnum::
KURLViolation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// META: global=window,worker
let code = new Uint8Array([0x53, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0]);
async_test(t => {
self.addEventListener('securitypolicyviolation', t.step_func_done(e => {
assert_equals(e.violatedDirective, "script-src");
assert_equals(e.originalPolicy, "default-src 'self' 'unsafe-inline'")
assert_equals(e.blockedURI, "wasm-eval")
}));
}, "Securitypolicyviolation event looks like it should");

promise_test(t => {
return promise_rejects_js(
t, WebAssembly.CompileError,
WebAssembly.instantiate(code));
});



Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Content-Security-Policy: default-src 'self' 'unsafe-inline'
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Verifies that CSP issue is created from a page with eval() usage.

Inspector issue: {
issue : {
code : ContentSecurityPolicyIssue
details : {
contentSecurityPolicyIssueDetails : {
contentSecurityPolicyViolationType : kWasmEvalViolation
isReportOnly : false
sourceCodeLocation : {
columnNumber : 19
lineNumber : 7
scriptId : <string>
url : https://devtools.test:8443/inspector-protocol/resources/content-security-policy-issue-wasm-eval.php
}
violatedDirective : script-src
}
}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
(async function(testRunner) {
const {page, session, dp} = await testRunner.startBlank(
`Verifies that CSP issue is created from a page with eval() usage.\n`);

await dp.Network.enable();
await dp.Audits.enable();
page.navigate('https://devtools.test:8443/inspector-protocol/resources/content-security-policy-issue-wasm-eval.php');
const issue = await dp.Audits.onceIssueAdded();

testRunner.log(issue.params, "Inspector issue: ");
testRunner.completeTest();
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php
header("Content-Security-Policy: script-src 'self' 'unsafe-inline';");
?>
<!DOCTYPE html>
<html>
<body>
<h2>Webpage with not allowed WebAssembly</h2>

<script>
const wasm_script = new Uint8Array([0, 0x61, 0x73, 0x6d, 0x1, 0, 0, 0]);
WebAssembly.instantiate(wasm_script);
</script>
</body>
</html>

0 comments on commit 15ea6f7

Please sign in to comment.