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

XSS vulnerability #235

Open
mhuggins opened this Issue Jan 25, 2015 · 2 comments

Comments

Projects
None yet
3 participants
@mhuggins

mhuggins commented Jan 25, 2015

I just tried implementing this in conjunction with bootstrap-markdown, and one of the first things I tested was how JS is stripped from links. It appears that links are not sanitized for JS input. Take the following example:

[link](javascript:alert(alert))

This is being converted to:

<p><a href="javascript:alert(alert)">link</a></p>

Clicking on it shows a popup with the value of alert. This is obviously not a dangerous example, but a well-crafted string might be able to put site users in danger.

@ashb

This comment has been minimized.

Collaborator

ashb commented Jan 26, 2015

So this is a tough one - I know of some people using this module who want javascript: links preserved, but this does suck for user input.

One possible way around this is to add a filter using this technique https://github.com/evilstreak/markdown-js#more-options (using this to loop over all links, rather than this is XSS protection) - if we add something like this to markdown-js then it needs to be optional (but on by default is fine)

@evilstreak

This comment has been minimized.

Owner

evilstreak commented Jan 26, 2015

One of the features of Markdown we still don't support (through lack of care and attention by us maintainers, rather than anything else) is allowing raw HTML to be included. Given that we definitely should support that, and doing so would allow any malicious HTML at all through, I'm not sure about shipping XSS protection with this code base.

That said, some clear documentation about how to do thinks like link sanitisation, stripping out raw HTML, or just doing a yes/no validation on whether the parsed content is safe would be a Very Good Thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment