Closed fskreuz closed 6 years ago
csp is actually enabled by default since 0.8, I think. If you pre-parse your templates and don't use the string form of computeds, then there should be no eval (or function constructor) calls.
There csp option takes any expressions in a template and composes them into a function hash on the output template ast. That means your template is no longer json, so it can be transported as such. In practice though, embedding the template in your script works out pretty well, and the Ractive bin makes it reasonably simple.
I think Ractive's use of eval is actually only dangerous if you pass through any user supplied data to be evaluated as template. If the user can supply dangerous functions as data to your template, then they pretty much already own the page anyways.
I said user, but I meant malicious third party. I'm still really confused as to how a malicious third party is supposed to get access to inject code or data without already having been included on a page by the author.
There are situations when expressions are dangerous, but it's not exactly because of eval (i.e., pre-parsing won't help you). Let's say you have a CMS where you have various access roles for different users. You want to allow some of them writing posts with some simple formatting (i.e. no dangerous HTML), and, for whatever reason, also use simple variables (mustaches) inside such posts. This would be safe without expressions, but with expressions you allowed them to gain complete control of the front-end. Even worse, if you use server-side rendering, you gave them complete control of you server. There are many other situations where people maybe have access to front-end anyway, but not to the back-end.
@MartinKolarik ahhhhh. That makes sense. I've never encountered quite that situation, but I see how it could arise. In most of what I do, users with access to the admin-y bits of the frontend are the ones that have a vested interest in not having malicious code on a site.
I suppose another dangerous area would be access to @global
, which in a node environment would give you require
and all the goodies that go with it.
Perhaps we should add some control over that for backend rendering? Maybe also inject window
and global
arguments into functions generated from expressions as well? That should just mean adjusting the parser-allowed globals to be prefixed with window.
and having the injected window
object populated with just those globals.
I also remember something about accessing a constructor
being dangerous, but I forget exactly how.
Oh, the constructor
thing is only an issue if you have an unsafe-eval
CSP. 1..constructor.constructor("alert('hello')")()
=== Function("alert('hello')")()
, but that's what CSPs are for.
@global
seems bad but in fact you can already do everything with the 1..constructor.constructor("alert('hello')")()
thing. Only safe option is to completely disallow expressions when running ractive server-side with untrusted input - I thought there was already a way to do this but now I can't find anything related?
It's not an option so much as just using the runtime distribution. It seems like it would be pretty easy to disallow .constructor.constructor
in keypaths at the parser too.
What about .constructor["construct" + ({}).toString()[1] + "r"]
? Also, there are most likely other ways than .constructor.constructor
.
That's fair. There's probably no way to handle all of the escape hatches available to js. The reference expression could disallow the constructor access in that scenario, but there's nothing really to stop some other construction that uses a full on expression.
I suppose the safest thing would be to strongly suggest sandboxing on the server if there's any cause not to trust user-generated content as in the CMS scenario above?
I think it's fine to just say that not all escape hatches get covered, especially the intricate ones. We can just document what's being covered and ones to look out for. :D
Reliable sandboxing in nodejs is also close to impossible, which is why I think that if you need to use untrusted user-generated content, you should disallow expressions completely. Currently that means deleting them after parsing and using the runtime build.
IMHO all we need to do is make sure people understand the potential problems, and maybe add an option, say eval: false
, which would tell ractive to throw an error when it encounters an expression (I actually thought we already had this - wanted to suggest it long time ago but probably forgot).
Oh wow. I just looked up how easy it is to escape a node vm sandbox, and it's pretty easy.
How about an eval: false
option for the parser that just skips expressions? Any template including calls, expressions, or anything not a plain reference would be a parse error.
EDIT: @MartinKolarik beat me to it 😄
How about an eval: false option for the parser that just skips expressions?
🎉
Any thoughts on the updated content?
Because security-related. 👮
Reworded the thing. Just making sure it's worded correctly. I'm under the impression that
eval
-like APIs used by Ractive are only in danger if it's fed untrusted data, which is something only a developer can introduce and only a developer can prevent.Plus I believe there's the
csp
initialization option, which I haven't looked into yet. Will try it out later and write about it in the same page.List of things mentioned in thread and to be added:
csp
is enabled by default.Ractive.parse
parses expressions as functions.