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

XSS vulnerability #454

Closed
LeaVerou opened this issue Nov 7, 2017 · 1 comment
Closed

XSS vulnerability #454

LeaVerou opened this issue Nov 7, 2017 · 1 comment

Comments

@LeaVerou
Copy link

LeaVerou commented Nov 7, 2017

Hey, thanks for making this wonderful library!
I’ve written a Mavo plugin based on it and recently someone brought the following vulnerability to my attention:

<img src=123 onerror="alert('foo')" />

This displays an alert immediately as the Markdown gets loaded, it doesn't even require a click or anything.
You do filter out <script>, so I suspect this is a concern to you.

@tivie
Copy link
Member

tivie commented Nov 7, 2017

@LeaVerou Thank you.

We've written an succint but complete article about Markdown's XSS Vulnerability (and how to mitigate it) which I recommend you read (really, it's not that long and can help you protect your project).

Also, showdown does not filter out script tags (you can check that in this fiddle). That's actually a browser feature that removes script tag insertion when setting innerHTML (and other DOM manupulation methods).

To summarize:

Showdown tries to convert the input text as closely as possible, without any concerns for XSS attacks or malicious intent. So, the basic rules are:

  • removing HTML entities from markdown does not prevent XSS. Markdown syntax can generate XSS attacks.
  • XSS filtering should be done AFTER Showdown has processed any input not before or during. If you filter before, it’ll break some of Markdown’s features and will leave security holes.
  • perform the necessary filtering server-side, not client side. XSS filtering libraries are useful but shouldn't be used blindly.

@tivie tivie self-assigned this Nov 7, 2017
@tivie tivie closed this as completed Nov 11, 2017
@tivie tivie mentioned this issue Nov 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants