Adding a few more badcharacters

[mrbrutti]

It would be interesting to contemplate adding even more characters.

[kpdecker]

This method is intended for escaping HTML content only, not javascript. Could you show me an example of what you are using this for that you'd like this change?

[mrbrutti]

Well, I was adding a few more cases to add common special characters that most libraries include. With that said, I found this when I was testing this edge case <a href={{foo}}/> when { 'foo' : 'test.com onload=alert(1)'} which will be translated as <a href=test.com onload=alert(1)/> which will end on a XSS vulnerability.

[kpdecker]

Ping me at kpdecker at gmail with the repo case.
On Tue, Aug 25, 2015 at 3:39 PM Matias P. Brutti notifications@github.com
wrote:

Well, I was adding a few more cases to add common special characters that
most libraries include. With that said, I found this when I was testing
this edge case when { 'foo' : 'test.com onload=alert(1)'} which will be
translated as which will end on a XSS vulnerability. http://test.com

—
Reply to this email directly or view it on GitHub
#1083 (comment)
.

[kpdecker]

Sorry that was formatted oddly in my email. No need to ping. This isn't the right fix as you are escaping to a different language and thus creating invalid data. Escaping = would be sufficient to mitigate, no?

[mrbrutti]

I was going to send another patch escaping ‘(‘ , ')’ and ‘=‘. The reason why I added the other special characters as well is because technically those should be escaped as well, but it could potentially break things if someone is trying to paste something on <p>{{text}}</p> which has \n. Although that will have to be a design decision from your library. 👍

With that said, technically this is more of a fault from developers not inserting the correct “ or ‘ to contain the value of the html Attribute, but it is such a common usage that I think you should try to fix it, by escaping a few more characters as you mentioned and I have recommended above.

[kpdecker]

Thanks for bringing this up. I went with the = escape as that seemed like the least amount of risk for users. In the compatibility notes I will also note that users should generally opt for quoted attributes whenever possible.