Lack of output encoding creates unavoidable security issues for .sub files

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41 by @hillbrad on 21 Jul 2014, 19:54 UTC:

Template substitution is a nice way to avoid "active" content like .py or .php files where security errors can give server-side code execution, but without an encoding context it introduces unavoidable XSS and other injection issues into the .sub files. (as compared to PHP where you can apply a sanitization function to data before including it into a page)

Consider example.sub.js:

var userData = "{{GET[userData]}}";

I can GET /example.sub.js?%22%3B%20var%20evil%20%3D%20%22somethingBad

Which yields:

var userData = ""; var evil = "somethingBad";

The same issue exists for .html, .sub.headers, etc...

It is probably necessary to add syntax and functionality to the template engine to describe the context into which the substitution will occur so that it can be properly escaped.

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49664951 by @hillbrad on 21 Jul 2014, 21:01 UTC:

It seems a common syntax for this in frameworks like Django and Jinja is to use a pipe character to allow specification of a chain of filters to apply to the template variable, e.g:

{{GET[unsafe]|jsescape}} or {{GET[unsafe]|htmlescape}}

or

{{GET[unsafe]|stripnewlines}}

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49668290 by @odinho on 21 Jul 2014, 21:27 UTC:

Possibly you meant this already, but those frameworks usually escape by default. Which is much smarter. I also like their dot-syntax better, but that's housing for bicycles.

{{ GET.unsafe }}

Could by default htmlencode all the things. Even ' and " and \ and ;.

Then you would do the same things to opt in to certain escaping other places:

{{ GET.unsafe|jsescape }}
{{ GET[unsafe]|htmlescape }}

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49668978 by @odinho on 21 Jul 2014, 21:33 UTC:

Also, what vectors do this really entail? w3c-test.org is not a normal website, it does not have any extra trust other thas some other random website on the internet. People could just as well use jsbin or Hixie's paste-thingy. I know that at least many leakage flaws are not that interesting because there isn't anything to leak.

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49670542 by @jgraham on 21 Jul 2014, 21:46 UTC:

It already HTML escapes by default.

I am considering adding other escaping modes, but for correctness rather than security. Given that the test server doesn't store any user information, it's not clear exactly what the security risk is. Certainly I can't think of any obvious attacks that couldn't otherwise be performed by getting a user to click on a form button, or similar.

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49674071 by @hillbrad on 21 Jul 2014, 22:19 UTC:

I suppose so. I'm continuing on anyway because I don't have time to fix any more framework issues that aren't showstoppers right now before I have to be ready for TTWF in < 2 weeks.

I just don't like it. I see my submissions dinged and held up by reviewers for things that really don't matter like trailing whitespace in a markdown file or using tabs vs. spaces -- the code quality issue I care about is security.

I feel bad inviting people to come learn and contribute to CSP tests with the idea that they'll learn to prevent injection flaws, only to tell them to use an idiom that is vulnerable.

And I don't want to run vulnerable code on my own website (where I mirror these tests, but also host other things) either.

Also, test code is example code. I like to set a good example.

On Mon, Jul 21, 2014 at 2:46 PM, jgraham notifications@github.com wrote:

It already HTML escapes by default.

I am considering adding other escaping modes, but for correctness rather than security. Given that the test server doesn't store any user information, it's not clear exactly what the security risk is. Certainly I can't think of any obvious attacks that couldn't otherwise be performed by getting a user to click on a form button, or similar.

— Reply to this email directly or view it on GitHub https://github.com/w3c/wptserve/issues/41#issuecomment-49670542.

[wpt-issue-mover]

Originally posted as https://github.com/w3c/wptserve/issues/41#issuecomment-49712372 by @odinho on 22 Jul 2014, 08:39 UTC:

@jgraham: Well, since it html escapes by default, that escaping could become more strict so it would also mess up javascript injection.

@hillbrad I hope you run it at a different origin at least. I think tests are primarily to check that implementations behave correctly. I wouldn't use it as example code. In fact just because you should do the minimal required to test what you want to test. And so you should take many shortcuts and try to test small parts by themselves.

About the other things, that would be much simpler if GitHub allowed us to change other people's pull requests. I don't like getting tabs and red whitespace in, but I could just as easy fix those things whilst reviewing, had it not been for GitHub's rather non- collaborative way of working on outside Pull-Requests.