Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify interaction between unsafe-eval and TrustedScript. #143

Closed
mikesamuel opened this issue Apr 3, 2019 · 24 comments · Fixed by #170
Closed

Clarify interaction between unsafe-eval and TrustedScript. #143

mikesamuel opened this issue Apr 3, 2019 · 24 comments · Fixed by #170

Comments

@mikesamuel
Copy link
Collaborator

@lweichselbaum @koto

Per https://twitter.com/we1x/status/1113340867409076224 since TT with eval/Function guards provides similar protection to CSP without unsafe eval, maybe it's worth clarifying how an application might provide degraded service when TT is not available but take advantage of TT where it is.


Should a CSP policy without unsafe-eval prevent eval(myTrustedScriptValue)?

I'm leaning no since unsafe-eval in a CSP header can't be contingent on trusted-types being supported.


If unsafe-eval is present, does trusted-types guard eval(x)/Function(x)/import(...) at all?


Similarly for wasm-unsafe-eval?

@lweichselbaum
Copy link

These are interesting questions!
On the one hand it would be nice to have a way to allow "trusted" eval via trusted types as it would simplify the rollout of a CSP that blocks eval (currently we have an all or nothing approach).
On the other hand it seems to make things more complicated if you'd suddenly start allowing the use of eval() although it's explicitly blocked by CSP.

I guess it would be less of a problem if trusted-types would be capable of guarding eval independently of CSP.

@mikesamuel
Copy link
Collaborator Author

Yeah. I think the core ambiguity is, with TT there's no difference between

  • Presence of unsafe-eval means eval(x) allowed for all x
  • Absence of unsafe-eval means eval(x) allowed for no x

It seems clear to me that the presence of TT eval guards unambiguously means

  • The presence of Trusted-Types means eval(x) allowed for x in TrustedScript.

Litmus test: backwards compatibility

Adding Trusted-Types to the Content-Security-Policy header with a default policy that blesses no script values and with no explicit policy.createScript(...) calls should not make eval more permissive.

Litmus test: Explicit trust decisions respected

A Content-Security-Policy header with Trusted-Types and without unsafe-eval should allow eval(policy.createScript(x)).


If those two litmus tests make sense, that leads to the following flow for %eval%(x)

  1. If Type(x) is String, then
    1. Let defaultPolicy = ? GetTrustedTypesPolicy(calleeRealm, 'default').
    2. If defaultPolicy is not null, then
      1. Set x to ApplyPolicy(defaultPolicy, TrustedScript, x)

before proceeding with the permissions check.

Then we'd have to define HostEnsureCanCompileStrings(calleeRealm, x) to do the following:

  1. If Type(x) is String, then
    1. If there is no Content-Security-Policy, then return.
    2. If the Content-Security-Policy has unsafe-eval, then return.
  2. Else if x is a TrustedScript from the calleeRealm, then return.
  3. Throw a new EvalError.

That leads to the following interpretation of unsafe-eval:

The absence of unsafe-eval means eval(x) is denied when the post-processed value of x is a string.

@arturjanc
Copy link

To dumb this down a bit: are you saying that a document with a CSP which bans eval() and enforces the use of TrustedScript for script-like sinks would ban eval(String) but allow eval(TrustedScript)?

@mikesamuel
Copy link
Collaborator Author

mikesamuel commented Apr 3, 2019

@arturjanc, Sorry for the overlong writeup.

are you saying that a document with a CSP which bans eval() and enforces the use of TrustedScript for script-like sinks would ban eval(string) but allow eval(TrustedScript)?

Yes, but with one wrinkle.

When eval(string) is banned, but there's a default TT policy, the default policy is first given the chance to coerce string to TrustedScript.

This would allow:

//// Header content: Trusted-Types "default"

//// Sensitive setup code
TrustedTypes.createPolicy('default', {
  createScript(x) {
    if (x === 'return this;') { return x; }
    throw new Error(x);
  }
});

//// Non-sensitive legacy application code
const globalObject = new Function('return this;')();

@Sora2455
Copy link

Sora2455 commented Apr 3, 2019

One major problem is that this won't work with the polyfill - in browsers that don't support TT but do support CSP (90% of current browsers), the eval can't be run, and no amount of polyfilling will fix that.

Therefore, devs won't be able to use eval with TT unless they start sending 'unsafe-inline' - which depending on how far back browsers they need to support, could mean they send that flag for the next ten years, their systems less safe instead of more.

@mikesamuel
Copy link
Collaborator Author

@Sora2455 It's true that direct eval can't be polyfilled, but indirect eval can be as can direct uses of new Function:

function functionPolyfill(...args) {
  // Load code via DOM manipulation with <script>
}
functionPolyfill.prototype = Function.prototype;
Function = functionPolyfill;
// Do the same thing with Function.prototype.constructor that the realms polyfill does.

Previously discussed at #120

@koto
Copy link
Member

koto commented Apr 5, 2019

@lweichselbaum

I guess it would be less of a problem if trusted-types would be capable of guarding eval independently of CSP.

I'm not sure I understand this. TT have that capability. The only interaction between TT and CSP for now is the same header used for the delivery of the enforcement policies.

  • Edit: clarified my mistake
    Initially, I think we should respect the existing restrictions of unsafe-eval (i.e. no eval and friends would be allowed, even with TrustedScript in the presence absence of unsafe-eval alone) for backwards compatibility. In that model, apps would have to decide:
  • Content-Security-Policy: script-src 'unsafe-eval'; trusted-types myPolicy
    which only allows TrustedScript in TT-supporting browsers, and strings in legacy ones. The JS code still passes through a policy modulo non-polyfillable cases that would cause functional breakage the application in non-supporting browsers, so this should not be cause security regressions in general.
    Alternatively, we may provide a separate script-src keyword like eval-allow-trustedscript that makes it more explicit. This keyword is technically redundant, but makes it clear what's happening, even if one looks only at the script-src directive.

or

  • Content-Security-Policy: script-src https://no-unsafe-eval; trusted-types myPolicy
    which would prohibit eval in all CSP-supporting browsers.

@mikesamuel
Copy link
Collaborator Author

@koto

I think we should respect the existing restrictions of unsafe-eval (i.e. no eval and friends would be allowed, even with TrustedScript in the presence of unsafe-eval alone) for backwards compatibility.

I don't think that's required for backwards compatibility since pre-TT programs do not create TrustedScript values.

That was the idea behind:

Litmus test: backwards compatibility

Adding Trusted-Types to the Content-Security-Policy header with a default policy that blesses no script values and with no explicit policy.createScript(...) calls should not make eval more permissive.

@koto
Copy link
Member

koto commented Apr 8, 2019

What I meant was that we should not relax the CSP restrictions. The issue is you can create policies (and types through them) even without a TT header, so that would allow the authors to call eval where they previously could not.

@mikesamuel
Copy link
Collaborator Author

@koto, It sounds like you're agreeing with:

  • Absence of unsafe-eval means eval(x) allowed for no x

What about if I want to write a CSP policy so that:

  • It denies eval pre-trusted types, which leads to some degraded service.
  • On a browser with trusted-types, it allows eval for some patterns, some possibly via a default policy which passes 'return this' and safeAPI(wellFormedJSON).

Given that the polyfill will not support direct eval, it sounds like this case requires serving different headers to different browsers depending on their level of TT support so that unsafe-eval is present only when trusted-types is used.


The issue is you can create policies (and types through them) even without a TT header

I thought in the absence of any grants, createPolicy is closed.
I can see how leaving it open would be better for library code -- in the common case, they can create values, so code paths that expect TT APIs becomes the most field-tested.

I wonder whether there's a happy middle-ground:

  • the absence of unsafe-eval means eval(x) fails for all x unless trusted-types is specified and Type(x) is TrustedScript.

That's not pithy and it means that code paths that use eval with TrustedScript are not the most field tested.

@koto
Copy link
Member

koto commented Apr 8, 2019

  • the absence of unsafe-eval means eval(x) fails for all x unless trusted-types is specified and Type(x) is TrustedScript.

If we can get it into CSP spec - sure, but I'm not sure CSP editors would like this extra complexity (given that this behavior is defined in a context of two separate directives).

@koto
Copy link
Member

koto commented Apr 8, 2019

Actually, @arturjanc reminds that 'unsafe-eval' only kicks in for strings (at least in Chrome):

https://jsbin.com/qehejuvido/edit?html,output

which would elegantly allow us to enable eval(TrustedScript) without needing unsafe-eval.

However, Chrome's behavior does not conform to spec: https://tc39.github.io/ecma262/#sec-eval-x and indeed, Firefox blocks that eval :/

@koto
Copy link
Member

koto commented Apr 8, 2019

w3c/webappsec-csp#201

@koto
Copy link
Member

koto commented Apr 8, 2019

Heh, I just found tc39/ecma262#1495 which describes this neatly :)

@Sora2455
Copy link

Sora2455 commented Apr 8, 2019

I should also note that the new Function("return this") use case is covered by globalThis, and the eval(JSON) is well and truly covered by JSON.parse. Do modern script authors actually need eval for anything?

@koto
Copy link
Member

koto commented Apr 9, 2019

eval is still used quite often actually. Some frameworks use it to back a expression parser in their template systems e.g. VueJS. Allowing eval selectively (e.g. only in trusted components of those frameworks) with TT would unblock CSP adoption for those frameworks (FWIW, VueJS is not the best example, as there is a way to avoid eval usage there).

@Sora2455
Copy link

Sora2455 commented Apr 9, 2019

@koto But those frameworks currently can't use CSP and eval together - and won't until TT is ubiquitous. That means that they have no find 'no-eval' solutions for the CSP-supporting non-TT supporting browsers, and once that's done there's no point re-enabling eval.

@koto
Copy link
Member

koto commented Apr 9, 2019

Not quite. Developing a non-eval based solution is sometimes quite complex, and the only real gain is being "CSP with no unsafe-eval" compliance. What we're seeing is that this gain is not big enough to trigger the change.

Adding TT-support is, ot the other hand, trivial, and backwards-compatible - adopting that gives one a TT compliance and "CSP with no-unsafe-eval in TT-supporting browsers" compliance, with is already a lot.

The application could then ship CSP script-src 'unsafe-eval'; trusted-types framework-eval-policy - this would be secure in TT-supporting browser, and secure modulo eval-that-was-never-called-in-a-TT-browser in other browsers.

@mikesamuel
Copy link
Collaborator Author

Actually, @arturjanc reminds that 'unsafe-eval' only kicks in for strings (at least in Chrome):
...
However, Chrome's behavior does not conform to spec: https://tc39.github.io/ecma262/#sec-eval-x and indeed, Firefox blocks that eval :/


Heh, I just found tc39/ecma262#1495 which describes this neatly :)


Yeah, the consensus is that even though Firefox is correct, that difference is due to a spec bug. I am prepping https://mikesamuel.github.io/eval-in-order/ as a needs_consensus fix which would require Firefox to become consistent with Chrome in this regard.

allenwb commented 9 days ago

This is a specification bug introduced post ES6.

In the ES6 spec there are no observable actions that happen before the the type check on the argument. This behavior dates all the way back to the ES1 spec:
...
It seems like the current spec. has an (unintentional??) breaking change.

@mikesamuel
Copy link
Collaborator Author

@Sora2455 said

I should also note that the new Function("return this") use case is covered by globalThis, and the eval(JSON) is well and truly covered by JSON.parse. Do modern script authors actually need eval for anything?

@koto But those frameworks currently can't use CSP and eval together - and won't until TT is ubiquitous. That means that they have no find 'no-eval' solutions for the CSP-supporting non-TT supporting browsers, and once that's done there's no point re-enabling eval.

Yes, there are better alternatives to these patterns but they appear deep in widely used libraries.

I worry that upgrading dependencies to newer versions of these libraries would require the kind of multi-level coordination that might prevent any one party from adopting TT+CSP in large systems.

@mikesamuel
Copy link
Collaborator Author

@koto

  • the absence of unsafe-eval means eval(x) fails for all x unless trusted-types is specified and Type(x) is TrustedScript.

If we can get it into CSP spec - sure, but I'm not sure CSP editors would like this extra complexity (given that this behavior is defined in a context of two separate directives).

I'd like to explore this as an option, and it's probably better to start sooner than later.
Do we know whom to talk to?
Would that conversation happen on public-webapps or another forum?

@koto
Copy link
Member

koto commented Apr 9, 2019

That would be https://github.com/w3c/webappsec-csp/. @mikewest is the editor.

@mikesamuel
Copy link
Collaborator Author

Ok. I think he's on vacation. I'll try to wrangle 10 minutes FTF.

mikesamuel added a commit to mikesamuel/trusted-types that referenced this issue May 9, 2019
This reworks how `new Function(source)` and `eval(source)` are
checked against a CSP and Trusted Types policy.

It trusts TrustedScript when the relevant CSP policies and TrustedType
configurations agree on whether to enforce and the TrustedType
configuration places limits on policy creation.

It also changes the previous language to use *calleeRealm* instead of
*callerRealm* for consistency with other sinks.

> ```js
>   let f = new self.top.Function(source);
> ```
> In this case, the *callerRealm*'s Window is `self` and the
> *calleeRealm*'s Window is `self.top`.
> The Trusted Types portion of this algorithm uses *calleeRealm*
> for consistency with other sinks.
> ```js
>   // Assigning a string to another Realm's DOM sink uses that
>   // Realm's default policy.
>   self.top.body.innerHTML = 'Hello, World!';
>   // Using another Realm's builtin Function constructor should
>   // analogously use that
>   // Realm's default policy.
>   new self.top.Function('alert(1)')()
> ```

It also makes recent versions of `bikeshed` run without warnings.

Fixes w3c#143
Issue w3c#144
mikesamuel added a commit to mikesamuel/trusted-types that referenced this issue Jul 1, 2019
This reworks how `new Function(source)` and `eval(source)` are
checked against a CSP and Trusted Types policy.

It trusts TrustedScript when the relevant CSP policies and TrustedType
configurations agree on whether to enforce and the TrustedType
configuration places limits on policy creation.

It also changes the previous language to use *calleeRealm* instead of
*callerRealm* for consistency with other sinks.

> ```js
>   let f = new self.top.Function(source);
> ```
> In this case, the *callerRealm*'s Window is `self` and the
> *calleeRealm*'s Window is `self.top`.
> The Trusted Types portion of this algorithm uses *calleeRealm*
> for consistency with other sinks.
> ```js
>   // Assigning a string to another Realm's DOM sink uses that
>   // Realm's default policy.
>   self.top.body.innerHTML = 'Hello, World!';
>   // Using another Realm's builtin Function constructor should
>   // analogously use that
>   // Realm's default policy.
>   new self.top.Function('alert(1)')()
> ```

It also makes recent versions of `bikeshed` run without warnings.

Fixes w3c#143
Issue w3c#144
@lukewarlow lukewarlow reopened this Feb 21, 2024
@lukewarlow
Copy link
Member

I've reopened this and #221 to continue this discussion around 'unsafe-eval', trusted types and the potential for a new script-src value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants