helpers / handlebars-helpers

188 handlebars helpers in ~20 categories. Can be used with Assemble, Ghost, YUI, express.js etc.
http://assemble.io/helpers/
MIT License
2.22k stars 364 forks source link

Add big red warning that says handlebars-helpers are not safe with untrusted templates #298

Closed aalexgabi closed 6 years ago

aalexgabi commented 6 years ago

handlebars-helpers must not be used when templates are provided by untrusted user input

I was very happy when I found this project. I was looking for any notice about untrusted templates safety in the project readme and I did not find any so I started looking at the code.

Some helpers are very dangerous (https://github.com/helpers/handlebars-helpers/blob/HEAD/lib/fs.js#L29):

helpers.read = function(filepath, options) {
  return fs.readFileSync(filepath, 'utf8');
};

A note should be added in the project readme to make sure that users are aware they should not template user input with this library, or at least not with all helpers.

Ideally there should be a flag that allows registering only save helpers.

jonschlinkert commented 6 years ago

no idea what this is referring to. Are you suggesting that by using the read helper that: 1) the user doesn't realize they are in fact reading a file that the user created on their own file system, or 2) that the fs module in node.js, the system required to run this library, is itself unsafe? or 3) something else?

aalexgabi commented 6 years ago

Most templating languages/libraries ars safe by default meaning the template can only access data provided in it's rendering context.

This project provides ways of accessing data outside of rendering context.

For example if I create a newsletter application where the user can upload his csv file and send emails to each recipient I want to give the user full control over the email template so I might provide a textarea allowing the user to write his own email template.

If the user writes something like this, the user can read any file on the server (which might contain very sensitive data):

{{read /etc/passwd}}

Unsafe means that this project should be used only in applications where the templates are written by developers and not by users. In my option most people assume that template engines and helpers are safe and somebody might use this project without being conscious of the security risk.

To make people conscious of this risk I propose adding a warning in the project readme.

jonschlinkert commented 6 years ago

Most templating languages/libraries ars safe by default

Sounds like you found some hard facts somewhere, can you show me where you found that data? Any references would be great.

edit: to be clear, I'm interested in seeing the report you found that audited "most" templating languages/libraries. If you really meant to say, "code libraries should strive to be safe by default", I don't think anyone can disagree with you there. But I'm betting it's more likely that "most templating libraries" are anything but safe by default (we could probably limit the analysis to regex with catastrophic backtracking alone).

This project provides ways of accessing data outside of rendering context.

The fs helpers can't be used in the browser, given that neither webpack nor browserify has support for it. What "users" are you referring to?

edit: please show a concrete example of where you're actually using this in the real world, as well as a description of how your users would actually be able to use the fs helpers in an unsafe way. If you can do that, it would help me understand your point more and, of course, I'd be happy to add the disclaimer to the readme.

aalexgabi commented 6 years ago
Most templating languages/libraries ars safe by default

Sounds like you found some hard facts somewhere, can you show me where you found that data? Any references would be great.

edit: to be clear, I'm interested in seeing the report you found that audited "most" templating languages/libraries. If you really meant to say, "code libraries should strive to be safe by default", I don't think anyone can disagree with you there. But I'm betting it's more likely that "most templating libraries" are anything but safe by default (we could probably limit the analysis to regex with catastrophic backtracking alone).

I did not do a study. This is from personal experience. Let's say most templating engines I have seen are safe by default. Some examples: handlebars, mustache, twig.

This project provides ways of accessing data outside of rendering context.

The fs helpers can't be used in the browser, given that neither webpack nor browserify has support for it. What "users" are you referring to?

edit: please show a concrete example of where you're actually using this in the real world, as well as a description of how your users would actually be able to use the fs helpers in an unsafe way. If you can do that, it would help me understand your point more and, of course, I'd be happy to add the disclaimer to the readme.

I'm developing a tool that allows sending templated api requests. For example the user uploads 100 objects that are processed by the server and then THE SERVER loops over them and for each one sends a REST HTTP request. Handlebars is used for templating the method, url, headers and body and the object being processed is passed in the context. fs module is available on the server so a user could read any readable file on the system.

I just thought it would be nice to warn users about this.