Kitura-TemplateEngine should escape XML entities by default

[NocturnalSolutions]

…unless the template engine implementation will do it, as defined by a parameter on the TemplateEngine class implementation. (IIRC, Mustache does it by default, but Stencil does not.)

That is, ", ', <, &, and >. Perhaps the other HTML entities, too, but at least those ones.

Why?

• It encourages security by default.
• It abstracts away a difference between template engine implementations. I will have to do my own escaping when I use Stencil, but if I later decide to switch to Mustache, I will have to remove that escaping or else my variables will be double-escaped. On the other hand, if I'm using Mustache and depending on Mustache's escaping, then later decide to switch to Stencil, my site becomes vulnerable to XSS exploits until I implement my own escaping, and I may never even realize it's necessary to do so.
• …but more importantly, it encourages security by default!

This could be implemented by simply mapping throughcontext and performing escaping on anything in there we find that looks like a String.

We can implement a new InsecureUnescapedString struct which users can use when they do not want a particular string in their context dictionary to be escaped; also, an option in RenderingOptions could be set if the user wants to override the default behavior for the whole dictionary (that is, they could force escaping to happen even if the template engine says it shouldn't, or vice versa).

I'm willing to put in the work to implement this and submit a PR, but I figured I'd float the idea out there first.

[Andrew-Lees11]

Just to let you know we are aware of this issue and are currently looking into solutions. Normally we would expect the template language to escape HTML entities like mustache does. We don't want to add double sanitation be adding it to this template engine. We could implement it for the Stencil-templateEngine but would prefer if Stencil did it be default. There is a pull request where your fix is suggested in Stencil but this is over a year old and in it Kyle discusses not having characters escaped by default.

We will continue looking into this and get back to you when we have decided on the best approach since you are right that customers shouldn't need to implement their own escaping.

[NocturnalSolutions]

I proposed something to avoid double-escaping in my OP, but I breezed over it, so let me go into more detail. We can add a new parameter to the TemplateEngine protocol named something like doesEntityEscaping where, if false, Kitura-TemplateEngine will do its own escaping before passing the strings to the implementation; otherwise it will pass the raw strings and let the implementation handle it.

This would be a backwards compatibility break, but a fairly easy one to rectify. And maybe there's a better way, but that's what I had in mind, at least.

[DunnCoding]

@NocturnalSolutions I realise it's been a while since a comment was made here but I'm going to look into resolving this issue.

Once I have something working I'll drop a link in here for you to take a look at. Thanks again for raising this issue!

[DunnCoding]

So I've spent some time exploring a possible avenue for implementing this into Kitura-TemplateEngine. It's possible but not 'clean' and it's difficult to add a case for a user not wanting to escape certain elements in their html but escape everything else. It'd be an escape all or nothing approach.

However this is a valid issue, so what we've decided to do is to reach out to Kyle and offered to work with him to bring this functionality into Stencil.

This should at least bring the template engines we support more in line.

[NocturnalSolutions]

it's difficult to add a case for a user not wanting to escape certain elements in their html but escape everything else. It'd be an escape all or nothing approach.

Should things be escaped at the Kitura-TemplateEngine level, we could create a simple new struct with a single string property called something like InsecureUnescapedString which the escaper would know not to escape, but just to print its property outright. Everything else that looks like a string would be escaped.

I still think it's a good idea to do this at the Kitura-TemplateEngine level for consistency's sake, since now we have a case where Mustache may escape some things that Stencil does not and vice versa, and who knows what new template engines that might come along in the future will do. That being said, Stencil seems to be the most commonly-used engine at present, so having that one be more secure by default would still be a big win.