URLs are improperly escaped

[JakobKallin]

Converting the following Markdown:

http://example.com/'onmouseover='alert(document.cookie)'/

yields the following HTML:

<p><a href='http://example.com/'onmouseover='alert(document.cookie)'/'>http://example.com/'onmouseover='alert(document.cookie)'/</a></p>

which in turn is parsed by the browser as:

<p><a href="http://example.com/" onmouseover="alert(document.cookie)" '="">http://example.com/'onmouseover='alert(document.cookie)'/</a></p>

The reason is that URLs are improperly escaped before being inserted into href attributes.

I discovered this by accident while writing a tutorial on XSS. It's a defect in the Showdown library that would allow for unintended script injection if Markdown weren't designed to allow inline HTML to begin with.

[tivie]

This is a necro issue, but I got the time to play around with showdown and XSS.

In short, my conclusion is that you can't really prevent XSS attacks with markdown and claiming to do so will probably give some sense of false security, which is worse.

The 2 rules of thumb are:

• filter for XSS after Showdown has processed any input, not before
• preferably server side, not client side

This excellent post by Michael Fortin provices more in-depth explanation

[JakobKallin]

Correct. As I wrote in my original comment, Markdown already allows markup injection by design, so this doesn't introduce any (unintended) security issues. It's still a defect, though, causing invalid markup to be generated in some cases. To avoid it, URLs should be HTML-escaped before being inserted between attribute quotes.

[tivie]

@JakobKallin I agree. The problem is assuring that by URL encoding, well, URLs, we won't break the parser. I will look into it.

[JakobKallin]

Also keep in mind the difference between URL encoding and HTML escaping (neither of which has a standardized name as far as I know): URL encoding replaces characters that are not allowed in URLs, while HTML escaping replaces characters with special meaning in HTML. While I think both would work fine here in practice, HTML escaping is the correct one in this context.