jehugaleahsa / mustache-sharp

An extension of the mustache text template engine for .NET.
The Unlicense
306 stars 78 forks source link

Html Escaping and non-escaping: {{{ or {{& #16

Closed crwang closed 8 years ago

crwang commented 10 years ago

Hello, thanks for putting this together!

I'm testing this and trying to use it in a project, but I noticed that the variables don't seem to be HTML escaped by default.

Am I missing something or is this not supported?

Please let me know! This is pretty important for us since we need to HTML escape most of our variables to avoid XSS attacks.

http://mustache.github.io/mustache.5.html

Thanks, Chris

jehugaleahsa commented 10 years ago

Mustache# is a general-purpose text template engine. It doesn't make sense to escape in that case.

However, there is a workaround. First, take a look at the functionality in the System.Web.HttpUtility class. There are methods for encoding and decoding HTML. Next, the Generator class has an event called KeyFoundEvent. The event args has a Substitute property that will allow you to replace what gets inserted into the output. That should get you what you want.

crwang commented 10 years ago

Thanks, let me take a look at it and try to modify it! I'll keep you posted with what I come up with.

crwang commented 10 years ago

Hi Travis,

I've gone into the code and have added html escaping, but I wanted to see if you want this is a default or not. The KeyFoundEvent is a workaround but is not core to the implementation. Ideally, we could integrate the html escaping into the core of what you're coded (in KeyGenerator.cs) since that is the mustache standard, but if you don't want to do this, then I could extend the FormatCompiler with an HtmlFormatCompiler class but may need to change some methods to protected.

Or, if you really don't want it in your core code base, I can integrate the code into the KeyFoundEvent, but also need to change a little bit in there to make it work.

Please let me know your thoughts!

crwang commented 10 years ago

Hi @jehugaleahsa ,

Did you see my comments about integrating in html escaping into the repo? Can you provide some feedback?

offtopic commented 10 years ago

@crwang,can you please share your modifications for escaping support?

crwang commented 10 years ago

@offtopic Sorry, I've been busy and haven't touched this in a long time.

I just forked it and copied my code and pushed it up there. I don't remember what state it's in, but I do know I coded in html escaping and am using it now.

https://github.com/crwang/mustache-sharp

jehugaleahsa commented 10 years ago

Could someone please try to explain to me what this issue is about? I think the name of the original issue is throwing me off. Is it just a matter of escaping the HTML generated by the keys by default? I would be happy to create a property on the Generator for turning this on. I just want to make sure I'm chasing down the right thing.

crwang commented 10 years ago

Hi @jehugaleahsa, the standard for mustache/handlebars is that code in {{myVar}} is html escaped by default but that if it has 3 braces: {{{myVar}}} then it doesn't format the code. This is to avoid formatting issues but especially problems with cross-site attacks and javascript injection.

so if myVar = "" then in the myVar example, the output should be:

{{myVar}}

<script>alert("1");</script>

But if it's

{{{myVar}}}

Then, the standard output should be:

<script>alert("1");</script>

So, basically, unless it's html escaped as according to the Mustache/Handlebar standard, then, the current code allows for attacks.

mitja-p commented 10 years ago

Can you add standard Mustache support? This includes as stated above {{var}} for escaped text and {{{var}} for unescaped text. And also {{#section}} support as alternative to {{#each section}}. For what is standard Mustache see: http://mustache.github.io/. Most projects use this standard and then only extend on it. Handlebars works with the standard template as well.