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

Is template context is fully sandboxed? #1048

Closed
slavafomin opened this issue Jan 6, 2018 · 5 comments
Closed

Is template context is fully sandboxed? #1048

slavafomin opened this issue Jan 6, 2018 · 5 comments

Comments

@slavafomin
Copy link

Hello!

Thank you for this great library!

Pardon me for my question, but it has a serious security implications to our project, so I really have to ask it.

We want to give access to templates editing to our customers, so they can customize their messages. We are passing some predefined variables as a context to render function and we expect that Nunjucks will only expose the passed context and nothing else. Is it really safe to give users the ability to edit the templates this way? Could it be used as an attack vector somehow?

For example, we pass some private keys and security tokens via environment variables to the node.js application. If some attacker could access the process.env object by constructing some special Nunjucks template, this would be a disaster.

Could you please elaborate on this problem?

Thank you!

@devoidfury
Copy link
Contributor

Nunjucks is not safe for this use, it's trivial to escape the main context and access process, require, and so on. As far as I'm aware, there is no safely sandboxed template engine in javascript.

@slavafomin
Copy link
Author

@devoidfury Thank you for your response. However, could you please demonstrate an interpolation attack of this kind? I just want to see it for myself.

If Nunjucks is really not secure in this respect, looks like we will have to implement our own secure string interpolation engine with support of a small subset of Jinja syntax.

@devoidfury
Copy link
Contributor

devoidfury commented Jan 8, 2018

{{ (0).toString.constructor("return global.process.versions")() | dump | safe }}

This will spit out the node versions, for example. The problem being that all functions have a constructor that eventually leads to Function, which is basically the same as eval and runs in toplevel scope. And keep in mind that most of that can be built with strings, so looking for constructor or return won't be enough, eg cycler['con'+'foso'[2]+'tructor'] will work just as well.

edit: this is due to nunjucks using a "compile-to-js" backend; all of the template code is compiled to regular javascript before rendering. most of the popular engines use a similar approach, or have a way to directly embed javascript -- including but not limited to pug, ejs, dot.

@fdintino
Copy link
Collaborator

Yes, this is not a good idea. That was the rationale behind the docs update in PR #1043. (Thanks for reminding me, I need to push out an update to the gh-pages branch so that the hosted docs get the update).

I made several aborted attempts at sandboxing, but was always able to come up with another way to bypass the sandbox. You could look into using something like vm2 and running the nunjucks api inside an isolated context.

@slavafomin
Copy link
Author

slavafomin commented Jan 11, 2018

Thank you for a thorough explanation! I think the better course of action would be to implement our own minimalistic engine for string interpolation then,

P/S: vm2 looks interesting too, I will study this option as well, thanks.

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

3 participants