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

Allow script-src 'unsafe-hashes' for eval() and new Function #623

Open
nicolo-ribaudo opened this issue Nov 3, 2023 · 8 comments
Open

Comments

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Nov 3, 2023

Right now the only way to allow eval (or the equivalent new Function) is by using the unsafe-eval policy. This policy should be used as little as possible, because it allows *any* usage of eval/Function rather than just a list of trusted ones.

One use case for eval is feature detection. When testing for new syntax you can wrap it in new Function and check whether it can be parsed or not. In this case strings are static, and so it's easy to hash them ahead of time and provide the has as part of the CSP:

let supportsDecorators = false;
try {
  new Function("@dec class A {}"); // allowed
  supportsDecorators = true;
} catch {}

import(supportsDecorators ? "./source.js" : "./transpiled.js");
</script>

another use case comes from the ShadowRealms TC39 proposal, that was recently demoted from Stage 3 to Stage 2 because the integration with the web platform is not fully flashed out yet.

That proposal exposes realms with a restricted interface to JavaScript, and there are two ways to run code in them:

  • synchronously, with a ShadowRealm.prototype.evaluate(code: string) method
  • asynchronously, with a ShadowRealm.prototype.importValue(moduleURL: string) method

.evaluate basically runs eval() inside the shadow realm, and for this reason it should follow normal CSP rules. However, under the existing policies this means that the only way to synchronously evaluate code in shadow realms is by using unsafe-eval.

How do you feel about extending hash-based policies to also support eval/new Function? This would make it possible to write the example above as follows:

<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-hashes' 'sha256-c25cd64187ae9eb96a23e52e9f55dfc4cf0fbf19f60885eca8abb94c9644a72a'">

<script>
let supportsDecorators = false;
try {
  new Function("@dec class A {}"); // allowed
  supportsDecorators = true;
} catch {}

import(supportsDecorators ? "./source.js" : "./transpiled.js");

new Function(someOtherString); // still disallowed!
</script>

and would work similarly with shadow realms.

An alternative, if we don't want to change the existing unsafe-hashes behavior, could be to introduce a new unsafe-eval-hashes policy.

(cc @ptomato, who is working with me on this)

@mikewest
Copy link
Member

mikewest commented Nov 6, 2023

I'm confident that we've talked about this kind of thing before, but I'm not able to find a reference quickly this morning. @arturjanc or @lweichselbaum might recall? If there's a need for it, tying it to 'unsafe-hashes' doesn't sound terrible to me.

If we make this change, we'd likely want to couple it with #574 so we don't continue incrementally changing the meaning of the keyword.

Specifying the change will require a little work, though, as I think tc39/ecma262#938 remains outstanding.

@ptomato
Copy link

ptomato commented Nov 6, 2023

Specifying the change will require a little work, though, as I think tc39/ecma262#938 remains outstanding.

I'm planning to rebase that PR and get the ball rolling again.

@arturjanc
Copy link

I'm also fairly sure we discussed hashes for eval() in the past, but I can't remember where (though I don't think we'd fully fleshed it out before, so it's not we're losing much context).

Extending the "powerful hashes" concept from #574 and #575 to apply to arguments to eval() sounds appealing to me. This would not only improve the adoptability of CSP, but also allow sites which use 'unsafe-eval' today to improve the security of their policies by substituting the global exception for eval() with a more granular allowlist.

The one remaining difficulty here is figuring out how to do this in a backward-compatible way; just expanding 'unsafe-hashes' would make policies using the keyword break in browsers that don't support the new, more flexible definition. So we might need a new keyword or directive here.

@mikewest
Copy link
Member

Easy, 'unsafer-hashes'.

ptomato pushed a commit to ptomato/ecma262 that referenced this issue Nov 16, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Nov 16, 2023

What do you think about calling them unsafe-eval-hashes and script-src-hashes/unsafe-src-hashes?

ptomato pushed a commit to ptomato/ecma262 that referenced this issue Nov 16, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
@nicolo-ribaudo
Copy link
Member Author

While thinking about the ECMAScript side of this, we realized that new Function is tricky.

new Function dynamically constructs the source code of a function and then evals it, so:

  • new Function(foo) is equivalent to eval(`(function () { ${foo} })`)
  • new Function(a, b, c, foo) is equivalent to eval(`(function (${[a,b,c].join()}) { ${foo} })`)

To make this actually usable, the CSP hash for new Function(foo) should be the hash of foo and not the hash of (function () { ${foo} }). This is doable, and it makes sense.

However, for new Function(a, b, c, foo) what could the hash be of? Maybe we should just support eval and Function with a single parameter, rather than also the variadic version of new Function.

ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 17, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks

Co-authored-by: Philip Chimento <pchimento@igalia.com>
ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 17, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks

Co-authored-by: Philip Chimento <pchimento@igalia.com>
@bakkot
Copy link

bakkot commented Nov 18, 2023

Maybe we should just support eval and Function with a single parameter, rather than also the variadic version of new Function.

That would be sufficient for feature testing, so that sgtm. If you want to make a function with parameters, you can always do new Function('let [a, b, c] = arguments; body'), so there's no real need for the variadic version.

It does have the downside of making it slightly harder to adopt for people who already have multiple-argument uses of new Function in their code base, but I expect those to be pretty rare.


Unrelated to the above, it might be a good idea to restrict unsafe-hashes to specifically the indirect version of eval. It's almost impossible to evaluate the security properties of a snippet when used with direct eval, since it depends on the surrounding context; a snippet which is safe in one context might do something completely different in other. This would hurt adoptability, though.

@nicolo-ribaudo
Copy link
Member Author

That would be sufficient for feature testing, so that sgtm. If you want to make a function with parameters, you can always do new Function('let [a, b, c] = arguments; body'), so there's no real need for the variadic version.

It does have the downside of making it slightly harder to adopt for people who already have multiple-argument uses of new Function in their code base, but I expect those to be pretty rare.

You can also always do eval("function (a, b, c) { body }"), which is maybe more intuitive. We might not need to support this in the function constructor at all, given the complexity it introduces with parameters.

Other than eval and new Function there is also setTimeout/setInterval, but they work the same as eval so they could be easily included.

Unrelated to the above, it might be a good idea to restrict unsafe-hashes to specifically the indirect version of eval. It's almost impossible to evaluate the security properties of a snippet when used with direct eval, since it depends on the surrounding context; a snippet which is safe in one context might do something completely different in other. This would hurt adoptability, though.

I would expect most usages of this feature to be with the string statically inside the eval() function, so that tools can statically detect which strings need to be hashed and passed to CSP. Tools wouldn't hash all the random strings that they find, so something like eval(someVariablePotentiallyDangerous) would probably not be allowed unless somebody goes out of their way to manually hash the string they expect and enable it.

ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 22, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks

Co-authored-by: Philip Chimento <pchimento@igalia.com>
ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 29, 2023
This is a continuation of Mike Samuel's work in tc39#1498. Due to tc39#2670, this
change can be made simpler than it previously was.

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

The original purpose of tc39#1498 was to support Trusted Types, which is still
a goal of this PR.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks

Co-authored-by: Philip Chimento <pchimento@igalia.com>
ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 29, 2023
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 30, 2023
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ptomato added a commit to ptomato/ecma262 that referenced this issue Nov 30, 2023
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ptomato added a commit to ptomato/ecma262 that referenced this issue Dec 14, 2023
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ptomato added a commit to ptomato/ecma262 that referenced this issue Dec 18, 2023
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ptomato added a commit to ptomato/ecma262 that referenced this issue Jan 9, 2024
This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
ljharb pushed a commit to ptomato/ecma262 that referenced this issue Feb 14, 2024
)

This change provides the source text to be evaluated, and the grammar
symbol that should be used to parse it, to the host hook
HostEnsureCanCompileStrings.

One example of where this is needed is for allowing a Content Security
Policy to provide hashes for code executed via `eval()` or
`new Function()`:
w3c/webappsec-csp#623
This is useful on its own, but has come up again in the topic of
ShadowRealm-HTML integration. In a ShadowRealm you can either execute code
asynchronously, with ShadowRealm.p.importValue, or synchronously, with
ShadowRealm.p.evaluate. Because the latter uses `eval()` inside the
ShadowRealm, it's subject to CSP rules, so the only CSP policy that will
let you execute synchronously in the realm is `unsafe-eval`.

This is a separate needs-consensus PR, rather than being part of the
ShadowRealm proposal, because it's useful independently of ShadowRealm,
and also ShadowRealm would go forward regardless of whether this goes
forward.

Prior art: https://github.com/tc39/proposal-dynamic-code-brand-checks
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

No branches or pull requests

5 participants