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

Bypass autoescape / XSS #835

Closed
matt- opened this issue Sep 7, 2016 · 1 comment
Closed

Bypass autoescape / XSS #835

matt- opened this issue Sep 7, 2016 · 1 comment

Comments

@matt-
Copy link

matt- commented Sep 7, 2016

The following string works as expected:

var res = nunjucks.renderString('Hello {{ username }}', { username: '<s>Matt</s>' });
console.log(res); //Hello &lt;s&gt;Matt&lt;/s&gt;

If however the variable passed to the template is an array autoescape does nothing:

var res = nunjucks.renderString('Hello {{ username }}', { username: ['<s>Matt</s>'] });
console.log(res); // Hello <s>Matt</s>

This looks to be intentional https://github.com/mozilla/nunjucks/blob/master/src/runtime.js#L209
However if a autoescape is on any variable that is rendered in a {{ }} block and appended to the output I would expect it to escaped no matter the type. If the var is just going to be part of a string concat anyway why not toString first then escape?

In express / Koa / (anything else using qs or body-parser) is is trivial to coerce query params types. See the following simple example in express:

var app = express();
var env = nunjucks.configure('views', {
    autoescape: true,
    express: app
});

app.get('/', function(req, res) {     
    // This should be fine autoescape is on...
    res.send(nunjucks.renderString('Hello {{ username }}', { username: req.query.name }));
    //res.render('index.html', { username: req.query.name }); 
});

http://127.0.0.1:3000/?name[]=<script>alert(1)</script>matt

I created a more detailed writeup and example app at: https://github.com/matt-/nunjucks_test

@devoidfury
Copy link
Contributor

Good catch, PR incoming.

devoidfury added a commit to devoidfury/nunjucks that referenced this issue Sep 7, 2016
devoidfury added a commit to devoidfury/nunjucks that referenced this issue Sep 7, 2016
devoidfury added a commit to devoidfury/nunjucks that referenced this issue Sep 7, 2016
vecmezoni added a commit that referenced this issue Sep 7, 2016
fix autoescape for non-string values; fixes #835
vecmezoni pushed a commit that referenced this issue Sep 7, 2016
vecmezoni added a commit that referenced this issue Sep 7, 2016
* origin/2.4.3:
  Bump version for 2.4.3 release.
  add credit to changelog
  fix escape filter #835
  update changelog
  use toString instead of shorthand casting
  add tests for cast xss
  fix autoescape for array values; fixes #835
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

2 participants