You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Include the string to be compiled in the call to HostEnsureCanCompileStrings #938
To improve the quality of CSP reports, it would be helpful for HostEnsureCanCompileStrings() to include the string to be compiled as an argument. HostEnsureCanCompileStrings(callerRealm, calleeRealm, source) seems ideal. :)
The goal is to ensure that we can include a sample of the script which violates the policy when generating a CSP violation report. We're doing this for inline <script>...</script> blocks today, and layering eval() and the like on as well would be helpful.
Sounds good in theory. The only reason we didn't do this at the time was because it wasn't needed, IIRC.
There are a couple of tricky things:
How do you want to handle non-strings? Currently in a CSP-restricted environment, eval(nonString) will throw. (I wonder if we have tests for that?) One refactoring would be to ensure things are a string before passing them to HostEnsureCanCompileStrings. If we do that, then behavior will change, and we'll bail out before ever hitting HostEnsureCanCompileStrings.
Similarly, for Function() and its various friends (GeneratorFunction(), AsyncFunction()), do we handle arg coercion before or after HostEnsureCanCompileStrings? This would change which error is thrown when doing e.g. Function({ toString() { throw 5; }).
For Function() and friends, what text should be passed? Should it be the body text of the function, which is directly passed as an argument? Or perhaps we should re-serialize the entire function, after we've assembled the various arguments to Function() into an actual function? The former would probably work for your purposes, although it's a bit weird to be doing checks on something that isn't standalone evaluatable source code. Maybe it would help if we named the new parameter something like sourceTextHint or proximateSourceText instead of just sourceText, with a reasonable explanation.
This issue also surfaced when creating the trusted types spec draft. In short, we're trying to figure out if eval could accept stringifiable objects, and the decision in HostEnsureCanCompileStrings would be based on their value (essentially, we would optionally reject all values that are not of a given type).
Actually, having looked at it, for Trusted Types in specific, we might need to be able to validate and coerce a given 'eval' argument to a string before e.g. PerformEval is called. The issue is that eval returns its input value if it's not a string:
eval(new String('foo'))
String {'foo'}
The use case we have in Trusted Types is to be able to, in the host environment, allow
Theoretically, this can still be done in HostEnsureCanCompileString if it accepted an object passed by the caller as-is and returned a tuple of boolean value and the (potentially coerced to string) code to compile. Spec-wise this would require small changes to a few HostEnsureCanCompileString callsites, namely:
Similarly, for Function() and its various friends (GeneratorFunction(), AsyncFunction()), do we handle arg coercion before or after HostEnsureCanCompileStrings? This would change which error is thrown when doing e.g. Function({ toString() { throw 5; }).
Excellent question.
If coercion happens too early, then HostEnsureCanCompileStrings lacks context to distinguish between a trusted script and a raw string.
If coercion happens after CanCompileStrings, then a HostEnsureCanCompileStrings implementation can't guarantee that the stringified form observes is the same as the stringified form that the parser sees.
It seems that for mkwst's diagnostic purposes, (2) is not a breaker, but for TT, (1) is a breaker.
Per the questions about functions, those seem like non-issues since, as koto points out, HostEnsureCanCompileStrings is never called.
$ npx node@10 --disallow_code_generation_from_strings -e 'console.log(eval(() => {}))'
[Function]
$ npx node@10 --disallow_code_generation_from_strings -e 'console.log(eval("() => {}"))'
[eval]:1
console.log(eval("() => {}")) ^EvalError: Code generation from strings disallowed for this context
Perhaps we could tweak the eval algo so that if the input is a string or has a particular symbol property then it is stringified to avoid breaking code that depends on the current behavior where eval is identity except for strings.
eval(` // External script, using by test 12, 8, 26, 135, 158, 160/sha256
document.getElementById("test_external_script").innerHTML = "Hi, I am script from external test.js";`)
And it cannot implement TrustedTypes either. The host needs to get the compiling source to determine if they should reject or allow this compilation.
@Jack-Works I agreed with you, in fact I was very surprised yesterday when looking at this. Maybe an implementer can provide more details about how this works today with trusted types? cc @mikesamuel
@nicolo-ribaudo I'm not sure this is actually addressed by that change. That change provides the strings separately but not the compiled version which is needed for CSP reports. This will be addressed by #3294 however.
Activity
domenic commentedon Jun 21, 2017
Sounds good in theory. The only reason we didn't do this at the time was because it wasn't needed, IIRC.
There are a couple of tricky things:
eval(nonString)will throw. (I wonder if we have tests for that?) One refactoring would be to ensure things are a string before passing them to HostEnsureCanCompileStrings. If we do that, then behavior will change, and we'll bail out before ever hitting HostEnsureCanCompileStrings.Function()and its various friends (GeneratorFunction(),AsyncFunction()), do we handle arg coercion before or after HostEnsureCanCompileStrings? This would change which error is thrown when doing e.g.Function({ toString() { throw 5; }).Function()and friends, what text should be passed? Should it be the body text of the function, which is directly passed as an argument? Or perhaps we should re-serialize the entire function, after we've assembled the various arguments toFunction()into an actual function? The former would probably work for your purposes, although it's a bit weird to be doing checks on something that isn't standalone evaluatable source code. Maybe it would help if we named the new parameter something likesourceTextHintorproximateSourceTextinstead of justsourceText, with a reasonable explanation.koto commentedon Jan 18, 2019
This issue also surfaced when creating the trusted types spec draft. In short, we're trying to figure out if
evalcould accept stringifiable objects, and the decision inHostEnsureCanCompileStringswould be based on their value (essentially, we would optionally reject all values that are not of a given type).koto commentedon Jan 24, 2019
Actually, having looked at it, for Trusted Types in specific, we might need to be able to validate and coerce a given 'eval' argument to a string before e.g.
PerformEvalis called. The issue is thatevalreturns its input value if it's not a string:The use case we have in Trusted Types is to be able to, in the host environment, allow
but optionally disallow:
or even transform it to :
Theoretically, this can still be done in
HostEnsureCanCompileStringif it accepted an object passed by the caller as-is and returned a tuple of boolean value and the (potentially coerced to string) code to compile. Spec-wise this would require small changes to a few HostEnsureCanCompileString callsites, namely:I'm not sure how involved the change is, both in spec and the implementations.
mikesamuel commentedon Jan 24, 2019
Excellent question.
It seems that for mkwst's diagnostic purposes, (2) is not a breaker, but for TT, (1) is a breaker.
Per the questions about functions, those seem like non-issues since, as koto points out, HostEnsureCanCompileStrings is never called.
Perhaps we could tweak the
evalalgo so that if the input is a string or has a particular symbol property then it is stringified to avoid breaking code that depends on the current behavior whereevalis identity except for strings.koto commentedon Aug 11, 2019
@mikesamuel, this is now merged into https://github.com/tc39/proposal-dynamic-code-brand-checks, right?
mikesamuel commentedon Aug 11, 2019
@koto, correct. https://tc39.es/proposal-dynamic-code-brand-checks/#sec-hostbeforecompilevalue
ljharb commentedon Aug 11, 2019
mikesamuel commentedon Aug 11, 2019
Jack-Works commentedon Aug 19, 2022
Without the source, the host cannot implement the following CSP:
And it cannot implement TrustedTypes either. The host needs to get the compiling source to determine if they should reject or allow this compilation.
caridy commentedon Aug 19, 2022
@Jack-Works I agreed with you, in fact I was very surprised yesterday when looking at this. Maybe an implementer can provide more details about how this works today with trusted types? cc @mikesamuel
koto commentedon Aug 19, 2022
nicolo-ribaudo commentedon Feb 22, 2024
The hook now receives the source: #3222
lukewarlow commentedon Mar 14, 2024
@nicolo-ribaudo I'm not sure this is actually addressed by that change. That change provides the strings separately but not the compiled version which is needed for CSP reports. This will be addressed by #3294 however.