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

Backtick is added to entityMap #388

Closed
wants to merge 1 commit into from
Closed

Backtick is added to entityMap #388

wants to merge 1 commit into from

Conversation

mtimur
Copy link

@mtimur mtimur commented Jul 1, 2014

Backtick is not being HTML encoded currently. I added it to entityMap.

Backticks may be used to perform XSS on some certain situations for IE8.0 clients, for e.g. http://html5sec.org/#102

@bobthecow
Copy link

But they won't be able to get html into the attribute anyway, since everything else is encoded, so I'm not sure how it could be exploited...

@bobthecow
Copy link

Internet Explorer treats any tag as plaintext in case the attribute delimiters are unbalanced

Okay, I'm going to assume you've got a template like this:

<a href="{{ link }}">click me!</a>

Let's say the haxors are trying to exploit it like this:

<a href="x` `onclick=alert(1)">click me!</a>

I could be wrong, but as I understand that exploit, Internet Explorer (lt 9) will treat it as if it were text, i.e. this:

&lt;a href="x` `onclick=alert(1)">click me!</a>

… which won't do anything sketchy. That's why their example injectect some htmls in there too:

<img src="x` `<script>alert(1)</script>">

Because IE (lt 9) would treat that as this:

&lt;img src="x` `<script>alert(1)</script>">

… which is, in fact, sketchy :)

In Mustache's case, since it's escaping the < anyway, the version with a script tag will be just fine. But like I said, I could be wrong. So I'm updating my oldIE VM so that I can spin it up and check :)

@mtimur
Copy link
Author

mtimur commented Jul 2, 2014

Backtick considered as a special character in some cases. You already mentioned one of those. Let me mention another one.

http://html5sec.org/#59

Internet Explorer treats the accent grave (`) as an attribute delimiter like " and '. The quotation mark (") will be stripped from the attribute value when using the innerHTML property in case it doesn't contain space.

<div id="div1"><input value="``onmouseover=alert(1)"></div> 
<div id="div2"></div><script>document.getElementById("div2").innerHTML = document.getElementById("div1").innerHTML;</script>

If you are interested and have time, you can also spend time studying this paper, https://cure53.de/fp170.pdf, which also talks around similar stuff.

What I suggest is, we should consider backtick as a special character. It would improve Mustache.js' security, won't affect usability and would make a good defense in depth example. Not for only those mentioned historical cases but also possible future ones.

@dasilvacontin
Copy link
Collaborator

Thanks for the report, @mtimur. I'll try to research the topic, read the paper, and follow up this PR.

phillipj added a commit that referenced this pull request Nov 17, 2015
This closes a couple of potential exploit scenarios.
Backtick (`) for older IEs and equals (=) for unquoted attributes.

Refs handlebars-lang/handlebars.js@83b8e84
Closes #388
@phillipj phillipj mentioned this pull request Nov 17, 2015
SteveDesmond-ca pushed a commit to SteveDesmond-ca/presentations that referenced this pull request Apr 19, 2017
prevents XSS vulnerability (see janl/mustache.js#388)
rastandy pushed a commit to rastandy/xwiki-presentation that referenced this pull request May 15, 2018
prevents XSS vulnerability (see janl/mustache.js#388)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants