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 dynamically enabling/disabling unsafe-eval #87

Open
devd opened this issue May 17, 2016 · 16 comments
Open

Allow dynamically enabling/disabling unsafe-eval #87

devd opened this issue May 17, 2016 · 16 comments
Milestone

Comments

@devd
Copy link

devd commented May 17, 2016

This allows us to limit these unsafe constructs to the cases where you know its happening.

@mikewest mikewest added this to the CSP3 CR milestone Sep 2, 2016
@mikewest
Copy link
Member

mikewest commented Sep 2, 2016

Hello, and welcome to the bug you filed several months ago that I'm only just now looking into. :)

How would you expect this to look and behave? I'm having trouble coming up with anything.

What's the back-compat story?

@mikewest
Copy link
Member

@wycats suggested on Twitter that this kind of thing is important for Ember and other frameworks. So let's take another shot at discussing it.

As a strawman, given a nonce-based policy:

eval('<exciting>template</exciting> goes here', 'nonce123')

Good enough?

@devd
Copy link
Author

devd commented Feb 19, 2017

This is fine too. But I think that it is easier to ask for the nonce for the API that enables/disables eval. Otherwise migrating old libs as well as asking current libs to pass nonce around for new browsers and support old browsers all becomes a mess. But I am fine with either; the approach you outlined is more secure but more painful to roll out IMO

@mikewest
Copy link
Member

Can you spell out what you're thinking in a little more detail? I'm not at all wedded to the strawman. :)

@devd
Copy link
Author

devd commented Feb 19, 2017

eval("alert(1)"); // fails
window.csp.enableEval('wrongNonce');//throws or maybe nothing happens
eval("alert(1)"); // still fails
window.csp.enableEval('nonce123');
eval("alert(1)"); //works
window.csp.disableEval();
eval("alert(1)"); // fails

But as I said even you strawman is fine. I defer to @wycats

@mikewest
Copy link
Member

It seems like that boils down to the same thing, just with more verbosity. :)

It would allow things like new Function() though, which I don't have a good idea for otherwise. Do y'all care about new Function() or setTimeout([string]) or any of the other string sinks?

@devd
Copy link
Author

devd commented Feb 19, 2017

well I think we will need a version of your strawman that allows new function and setTimeout with the right nonce.

@mikewest
Copy link
Member

And I think we need a version of your strawman that at least sketches out answers to the design questions we started addressing in https://w3c.github.io/webappsec-csp/api/ but with which we never got anywhere. :)

While we could make setTimeout/setInterval, etc. understand a nonce pretty trivially, we can't add things to new Function(...) without substantial hackery. If y'all need that mechanism in particular, then we need to do some more work together.

@mikesherov
Copy link

IM(layman)O, there are upsides and downsides to both approaches, but it seems as if we want new Function() to work, we need the window.csp.enableEval() approach. I'm wary of trying to introduce new functionality into the Function constructor, and forcing that through TC39 at the language level. Of course, this leaves open the notion of accidentally forgetting to disable when you're done with the eval.

@annevk
Copy link
Member

annevk commented Feb 20, 2017

Yeah, I don't think we can realistically expect to change eval() either. Changing this globally seems much more doable, though also less safe.

@mikewest
Copy link
Member

If changing eval() is out (though I guess I don't share your skepticism, @annevk, since my (hasty) spot-check of browsers shows that they consistently ignore a second argument), then we need an API. I'm a little worried about adding a one-off API without designing the framework into which that bit would fit, but at the same time, that's a lot of work that no one has taken up in the last ~5 years. So. Anyone interested? :)

@arturjanc: FYI.

@annevk
Copy link
Member

annevk commented Feb 20, 2017

My skepticism is about convincing TC39.

@mikewest
Copy link
Member

I see.

@andypaicu andypaicu modified the milestones: CSP3 CR, Future Jan 8, 2018
@arturjanc
Copy link

Now that we have a directive split of script-src into separate directives which can separately control script elements and attributes (explainer, Blink intent to implement), which allows whitelisting inline event handlers with hashes, one of the bits of feedback has been to extend this notion to eval().

Implementing something like script-src-eval to allow hashes to bless the arguments to eval() would make it possible to bless the evaluation of known static strings (commonly used for feature/version detection), while preventing the execution of arbitrary expressions. It is not as flexible as a full-fledged API or nonces, but it would mesh fairly well with the current design. It might also be useful in a world with Trusted Types which AFAIK don't restrict eval(), because it would allow some measure of control over dynamically evaluated expressions without requiring them to be banned outright ('unsafe-eval' is used in ~80% of policies deployed in the wild.)

@arturjanc
Copy link

One interesting aspect of introducing something along the lines of 'script-src-eval' as outlined above is that it could allow more granular control of eval()-like code in WebAssembly (#293).

A directive which governs all JS APIs which evaluate code at runtime (i.e. eval(), new Function(), ..., WebAssembly.instantiate(), ...) would let us avoid having a binary safe/unsafe switch, which is the main problem with 'unsafe-eval'. It seems to also address, at least partly, @andypaicu's concern in #293 (comment): since the unsafe-wasm-eval keyword would likely not be a transient opt-in to insecurity while developers refactor their applications (as was the hope with 'unsafe-eval'), this proposal would give us a more general mechanism to allow only explicitly trusted Web Assembly code.

My (uneducated) guess is that the arguments to WASM eval()-like functions are usually static so the hash-based approach could work well here.

@devd
Copy link
Author

devd commented Nov 21, 2018

heh .. just ran into this again today so pinging this thread to annoy Mike :D

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

6 participants