Consider warning about arbitrary code injection?
hexpunk opened this issue · 7 comments
Would you consider adding a warning about arbitrary code injection in your docs somewhere? Most people's workflows are probably fine, but if someone uses doT in such a way where the template comes from an untrusted source (i.e. the user), there's a risk of arbitrary code injection due to how the evaluation delimiters work (using the function constructor, a slightly safer form of eval).
A nefarious template from an untrusted source could even be used to execute arbitrary code located in the context data.
For example:
Template
<div>Hi {{ with(this){eval(it.name)} }}!</div>
<div>{{=it.age || ''}}</div>Data
{"name":"alert('I was injected!')","age":31}Why would anyone in their sound mind eval user-provided data?
The example you provided is usually written as <div>Hi {{! it.name }}!</div> if data is not trusted and embedded in HTML, or <div>Hi {{= it.name }}!</div> if data comes from a trusted source.
As I said, the risk comes from running templates from untrusted sources.
But even jade, ejs they do not have that kind of warning (as I remembered). doT in many way minimize the unnecessary compiling tasks, and it should only doing one job, i.e. compile your template to a JS function. I will say it's the user's responsibility to evaluate the source of file if is trustworthy. If you want to compile template from DB data, it should also running some sanitizer (e.g. express-sanitizer for express on node).
The argument against adding a warning in the documentation because other projects don't do it isn't a particularly strong one.
I personally think any Javascript library that uses eval or the function constructor should be upfront about any potential for code injection. It's not reasonable to expect users of the library to have to scan the code for vulnerabilities they may not even realize could exist.
Oh right, I completely misunderstood your point here. Compiling user-provided templates introduces the same risk as running arbitrary JS, so it's generally a bad idea. I think the only way to let the user provide custom templates is to use a logic-less engine such as Mustache etc. (I'm not sure if it's secure in that case though).
I can't speak for the maintainers, but I believe if you post a pull request for docs with a clear explanation why templates have the same control over user's environment as plain JS code, it has a high chance of being merged.
FYI, I was also able to confirm this vulnerability and yes, where the template comes from an untrusted source (i.e. user), they will be able to execute server-side code under the current Node.js context, including killing the owning process altogether.
added security considerations to docs. doT templates are code.