Commit 6c70c60e authored by Daniel Vogelheim's avatar Daniel Vogelheim Committed by Chromium LUCI CQ

[Trusted Types] Fix "sink" name for Function constructor.

Fix-it week: Fix the "sink" name for Function constructors. The original
implementation tried to keep it simple and disambiguated between "eval" and
"Function" only when the value was available. That wasn't quite sufficient,
so now we pass in the value whenever we determine the sink for script
values. (This follows the spec, as linked in the issue.)

Bug: 1153054
Change-Id: Ie53f9ee30428b6c94140cabf87d7027367b055a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584024
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: default avatarYifan Luo <lyf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835732}
parent 3279b116
...@@ -46,6 +46,12 @@ enum TrustedTypeViolationKind { ...@@ -46,6 +46,12 @@ enum TrustedTypeViolationKind {
kScriptExecutionAndDefaultPolicyFailed, kScriptExecutionAndDefaultPolicyFailed,
}; };
// String to determine whether an incoming eval-ish call is comig from
// an actual eval or a Function constructor. The value is derived from
// from how JS builds up a string in the Function constructor, which in
// turn is defined in the TC39 spec.
const char* kAnonymousPrefix = "(function anonymous";
const char kFunctionConstructorFailureConsoleMessage[] = const char kFunctionConstructorFailureConsoleMessage[] =
"The JavaScript Function constructor does not accept TrustedString " "The JavaScript Function constructor does not accept TrustedString "
"arguments. See https://github.com/w3c/webappsec-trusted-types/wiki/" "arguments. See https://github.com/w3c/webappsec-trusted-types/wiki/"
...@@ -99,7 +105,8 @@ const char* GetMessage(TrustedTypeViolationKind kind) { ...@@ -99,7 +105,8 @@ const char* GetMessage(TrustedTypeViolationKind kind) {
return ""; return "";
} }
String GetSamplePrefix(const ExceptionState& exception_state) { String GetSamplePrefix(const ExceptionState& exception_state,
const String& value) {
const char* interface_name = exception_state.InterfaceName(); const char* interface_name = exception_state.InterfaceName();
const char* property_name = exception_state.PropertyName(); const char* property_name = exception_state.PropertyName();
...@@ -110,7 +117,9 @@ String GetSamplePrefix(const ExceptionState& exception_state) { ...@@ -110,7 +117,9 @@ String GetSamplePrefix(const ExceptionState& exception_state) {
if (!interface_name) { if (!interface_name) {
// No interface name? Then we have no prefix to use. // No interface name? Then we have no prefix to use.
} else if (strcmp("eval", interface_name) == 0) { } else if (strcmp("eval", interface_name) == 0) {
sample_prefix.Append("eval"); // eval? Try to distinguish between eval and Function constructor.
sample_prefix.Append(value.StartsWith(kAnonymousPrefix) ? "Function"
: "eval");
} else if ((strcmp("Worker", interface_name) == 0 || } else if ((strcmp("Worker", interface_name) == 0 ||
strcmp("SharedWorker", interface_name) == 0) && strcmp("SharedWorker", interface_name) == 0) &&
!property_name) { !property_name) {
...@@ -139,12 +148,13 @@ const char* GetElementName(const ScriptElementBase::Type type) { ...@@ -139,12 +148,13 @@ const char* GetElementName(const ScriptElementBase::Type type) {
HeapVector<ScriptValue> GetDefaultCallbackArgs( HeapVector<ScriptValue> GetDefaultCallbackArgs(
v8::Isolate* isolate, v8::Isolate* isolate,
const char* type, const char* type,
const ExceptionState& exception_state) { const ExceptionState& exception_state,
const String& value = g_empty_string) {
ScriptState* script_state = ScriptState::Current(isolate); ScriptState* script_state = ScriptState::Current(isolate);
HeapVector<ScriptValue> args; HeapVector<ScriptValue> args;
args.push_back(ScriptValue::From(script_state, type)); args.push_back(ScriptValue::From(script_state, type));
args.push_back( args.push_back(
ScriptValue::From(script_state, GetSamplePrefix(exception_state))); ScriptValue::From(script_state, GetSamplePrefix(exception_state, value)));
return args; return args;
} }
...@@ -168,11 +178,7 @@ bool TrustedTypeFail(TrustedTypeViolationKind kind, ...@@ -168,11 +178,7 @@ bool TrustedTypeFail(TrustedTypeViolationKind kind,
if (execution_context->GetTrustedTypes()) if (execution_context->GetTrustedTypes())
execution_context->GetTrustedTypes()->CountTrustedTypeAssignmentError(); execution_context->GetTrustedTypes()->CountTrustedTypeAssignmentError();
const char* kAnonymousPrefix = "(function anonymous"; String prefix = GetSamplePrefix(exception_state, value);
String prefix = GetSamplePrefix(exception_state);
if (prefix == "eval" && value.StartsWith(kAnonymousPrefix)) {
prefix = "Function";
}
bool allow = bool allow =
execution_context->GetContentSecurityPolicy() execution_context->GetContentSecurityPolicy()
->AllowTrustedTypeAssignmentFailure( ->AllowTrustedTypeAssignmentFailure(
...@@ -266,7 +272,7 @@ String GetStringFromScriptHelper( ...@@ -266,7 +272,7 @@ String GetStringFromScriptHelper(
TrustedScript* result = default_policy->CreateScript( TrustedScript* result = default_policy->CreateScript(
context->GetIsolate(), script, context->GetIsolate(), script,
GetDefaultCallbackArgs(context->GetIsolate(), "TrustedScript", GetDefaultCallbackArgs(context->GetIsolate(), "TrustedScript",
exception_state), exception_state, script),
exception_state); exception_state);
if (exception_state.HadException()) { if (exception_state.HadException()) {
exception_state.ClearException(); exception_state.ClearException();
...@@ -371,7 +377,7 @@ String TrustedTypesCheckForScript(String script, ...@@ -371,7 +377,7 @@ String TrustedTypesCheckForScript(String script,
TrustedScript* result = default_policy->CreateScript( TrustedScript* result = default_policy->CreateScript(
execution_context->GetIsolate(), script, execution_context->GetIsolate(), script,
GetDefaultCallbackArgs(execution_context->GetIsolate(), "TrustedScript", GetDefaultCallbackArgs(execution_context->GetIsolate(), "TrustedScript",
exception_state), exception_state, script),
exception_state); exception_state);
DCHECK_EQ(!result, exception_state.HadException()); DCHECK_EQ(!result, exception_state.HadException());
if (exception_state.HadException()) { if (exception_state.HadException()) {
......
...@@ -35,6 +35,9 @@ ...@@ -35,6 +35,9 @@
_ => script.textContent = "2+2" ], _ => script.textContent = "2+2" ],
[ "about:blank", "TrustedScriptURL", "HTMLScriptElement src", [ "about:blank", "TrustedScriptURL", "HTMLScriptElement src",
_ => script.src = "about:blank" ], _ => script.src = "about:blank" ],
[ "2+2", "TrustedScript", "eval", _ => eval("2+2") ],
[ "(function anonymous(\n) {\nreturn 2+2\n})", "TrustedScript",
"Function", _ => new Function("return 2+2") ],
]; ];
for (var tc of cases) { for (var tc of cases) {
test(t => { test(t => {
......
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