-
Notifications
You must be signed in to change notification settings - Fork 93
Cross-realm eval() calls and 'unsafe-eval' #438
Description
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:
- Allow both
f.contentWindow.eval('foo')andf.contentWindow.Array('foo').map(f.contentWindow.eval). I.e.: stop checking callerRealm, check only calleeRealm. This makes the most sense IMHO. (FWIW, all current browsers implement it this way at the moment, not following the spec.) - Forbid both
f.contentWindow.eval('foo')andf.contentWindow.Array('foo').map(f.contentWindow.eval). This might be ugly. Say hello to incumbent realm?
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.