w3c / webappsec-csp

WebAppSec Content Security Policy
https://w3c.github.io/webappsec-csp/
Other
209 stars 78 forks source link

Cross-realm eval() calls and 'unsafe-eval' #438

Open TomiBelan opened 4 years ago

TomiBelan commented 4 years ago

The rules of cross-frame string compilation have some counter-intuitive results.

"If a parent frame forbids 'unsafe-eval' and a child frame allows 'unsafe-eval', and both are on the same origin, should childIframeElement.contentWindow.eval('foo') be allowed?"

The CSP spec says no. EnsureCSPDoesNotBlockStringCompilation at https://w3c.github.io/webappsec-csp/#can-compile-strings says that both callerRealm and calleeRealm must allow 'unsafe-eval'. (callerRealm is from the second to top element of the execution context stack, i.e. the parent iframe. calleeRealm is from eval's context, i.e. the child iframe. See https://tc39.es/ecma262/#sec-eval-x.)

I guess the intention of this rule might've been to force the iframes to intentionally cooperate in this scenario. For example, maybe the idea was to allow it only if the child iframe explicitly defines a global function that calls eval, and the parent iframe calls that global function, instead of calling contentWindow.eval directly.

However, the child iframe's cooperation is not required. The parent can run e.g. f.contentWindow.Array('foo').map(f.contentWindow.eval), and this is allowed by the spec, because the second to top stack element is the child iframe's native map method.

I don't think that it makes sense that f.contentWindow.eval('foo') is forbidden but f.contentWindow.Array('foo').map(f.contentWindow.eval) is allowed. If an XSS-vulnerable page can be tricked into running one with untrusted user input, I think it's likely that it can also be tricked into running the other. (Side note: a script gadget of this strength might just be able to run anything anyway, with createElement + append, especially if 'strict-dynamic' is also allowed.)

There are two possible directions for resolving this:

In other words: if the callerRealm restriction is pointless, it should be removed. If it is not pointless and fulfills an important purpose (which I can't see), it should be strengthened.

TomiBelan commented 4 years ago

There was an email thread at Google about this in early 2018. It didn't reach a conclusion (everyone was too busy at the time). @koto prompted me to finally file this issue about it.

I'm pasting the thread here, to continue the discussion and maybe to refresh people's memory. I removed some details about the Google project where this issue came up (it's not public). Ping me on corp if you need details about the use case.

Thread from 2018 Tomi: > If I'm reading the spec correctly, ?.contentWindow.eval() should not be allowed. https://www.w3.org/TR/CSP3/#ecma-integration checks that neither callerRealm nor calleeRealm has a policy that doesn't contain 'unsafe-eval'. https://tc39.github.io/ecma262/#sec-eval-x says that calling the global "eval" function sets calleeRealm to the current realm and callerRealm to (AFAICT) the next-to-last stack frame's realm. Do you know what's up with that? Is it a Chrome bug? Artur: > Interesting, I've never noticed this part of the spec. +Mike West do you have any thoughts about documents with CSP banning eval() being able to access it via a same-origin frame which doesn't set CSP ([this case](https://arturjanc.com/eval-alternatives/direct-eval-in-frame.html))? My guess is that it's not a Chrome bug because I see identical behavior in Firefox, Edge and Safari, but I wonder if it's because they're all inconsistent with the spec or if I'm misunderstanding the meaning of some of the terms. Mike: > It surprises me a little that grabbing `eval()` from a different window would work. +Domenic, who helped out with CSP's integration with ECMAScript, and who knows things about realms. Domenic: > So we definitely made the spec disallow that. However, I think we did so before some of the recent focus on making sure spec changes have appropriate web platform tests and implementation commitments. > > If someone is able to write a web platform test for these cases, and file bugs against all the browsers that don't implement it, that would be a great way to increase the security of the web. Tomi: > I have more questions for Domenic. Although browsers are doing the wrong thing, maybe the spec itself is not quite "correct" either! > > If I understand correctly, the spec says to check the two topmost stack frames (aka "execution contexts") - i.e. the frame of the "eval" itself, and the function that called it. So for example, if we have a parent which cannot call eval and a child iframe which can, the spec forbids `my_child_iframe.contentWindow.eval('123')`. > > A cooperating child could provide an escape hatch such as "`function helper(s) { return eval(s) }`". Then the parent would be allowed to call `my_child_iframe.contentWindow.helper('123')`. But it's more interesting when the child doesn't want that to happen. > > My first question is: Does the spec allow the parent to do `my_child_iframe.contentWindow.Array('123').map(my_child_iframe.contentWindow.eval)`? The two topmost stack frames are "eval" and "map", and both are from the child's realm. > > My second question is: *Should* it be allowed? > > If it should be allowed, then why not also allow `my_child_iframe.contentWindow.eval('123')`? If it should be forbidden, does that mean the spec has to change so that eval checks the policy of (gasp!) the [incumbent](https://html.spec.whatwg.org/#realms-settings-objects-global-objects) realm? It seems there is no good option. > > Tell me what you think. Artur: > Friendly ping -- Domenic, do you have any thoughts about this? > > From my point of view, there is relatively little security benefit in having CSP's restrictions on eval() apply across same-origin documents. If an application wants to offload the execution of eval() to an iframe, it already can do so without directly accessing it in the child, e.g. by instead communicating with it via postMessage and having the child eval() data sent in the message and return the results. > > Even if eval() is directly accessible via window.frames[0].contentWindow.eval, it is relatively unlikely that an injection into the parent would be able to call it to execute code -- or, in other words, if an attacker has sufficient control to call eval in the child frame, she is likely already able to execute code in the parent. Also, this would come into play only in applications which specifically frame a same-origin document with 'unsafe-eval' from a top document without 'unsafe-eval', which indicates that the application needs to use eval(); in those situations, the alternative is for the top document to just set 'unsafe-eval' instead, which would be strictly worse security-wise. > > In practice, I think both the status quo as implemented (no eval restrictions for same-origin cross-document eval), and the behavior as defined in the spec as interpreted by Tomi above (no direct access to eval, but ability to call a wrapper) are reasonable. They both give us an escape hatch that lets us remove eval() in the top document, but still access it if necessary via a child frame. [...snip...] Domenic: > I'm sorry, I don't really have time to review, research, and answer all of these detailed questions outside of my area of expertise. I'll let Mike take over, as he's responsible for the CSP spec and Chrome's implementation.
annevk commented 4 years ago

I tend to agree that trying to enforce cross-document consistency when documents (can) have different policies is not a good use of time.

cc @domenic

dinofx commented 3 years ago

Is there any browser that blocks unsafe evaluation when just a single frame is involved?

test.html:

<meta http-equiv="content-security-policy" content="script-src 'unsafe-inline'">
<script>
hash=location.hash.substring(1);
malicious='javascript:' + hash;
location.href=malicious;
//eval(malicious);
</script>

When I open this page in Chrome file:///Users/.../test.html#alert(malicious), the dynamic script built from the URL fragment is evaluated.

TomiBelan commented 3 years ago

@dinofx, I think that is not related to this issue. Your test.html is working as intended. According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src, 'unsafe-inline' allows javascript: URLs. If you want to forbid navigation to javascript:, don't use 'unsafe-inline'. See also https://w3c.github.io/webappsec-csp/#directive-script-src.

antosart commented 2 years ago

Given the discussion, and since vendors are already checking only calleeRealm (see #539), I think it makes sense to change the spec to do the same.

I already wrote tests, I'll draft a PR.

annevk commented 2 years ago

Will you also change JS and HTML? (This also came up in the Wasm discussion.)

domenic commented 2 years ago

I think technically no changes to JS and HTML are required. CSP would just ignore one of the arguments those specs pass. Which seems reasonable-ish.