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

User assisted XSS #976

Closed
chrisjohansson opened this issue Jun 5, 2020 · 3 comments
Closed

User assisted XSS #976

chrisjohansson opened this issue Jun 5, 2020 · 3 comments
Labels
3rd-party Should be implemented as a third party extension.

Comments

@chrisjohansson
Copy link

chrisjohansson commented Jun 5, 2020

Hi
Users can supply javascript: URIs when constructing Markdown links, example:
[click me for XSS]( javascript:alert(1) )
This can be abused by an attacker to supply arbitrary javascript that will execute if someone clicks the link. It would be desirable if one could supply an allowlist (for example http://, https://) to the parser (if such a feature already exists and I missed it I apologise).

Note, I originally filed this as a bug in Netbox (netbox-community/netbox#4717) since that was the codebase I was auditing, however the maintainers thought this would be better addressed in the Markdown parser (and they might have a point).
Kind Regards
Christian

@facelessuser
Copy link
Collaborator

This is not a direct concern for Python Markdown. It is quite customizable, and it is not really up to Python Markdown to choose how a user wishes to use it. In some environments, like in static documentation generation, the risk of a 3rd party exploiting this is low, in environments where a 3rd party can inject Markdown source to have it rendered, the risk is higher.

Depending on how a user wishes to use Python Markdown is what makes this issue serious or not. Some users may wish to safely inject JavaScript in their documents.

I would argue that It is up to the user to sanitize their content if it is a concern. There are projects, such as Bleach, that have been created to deal with these kinds of things. You could write a simple Markdown extension that sanitizes your content with such a project in a post process event. Or you could just run it on the content after the Markdown translation as a separate step (probably the best and safest approach).

If Netbox is concerned with having sanitized HTML, then I would argue they should take action to ensure their content is sanitized. I would argue you filed it in the correct place when creating at Netbox. For some, this behavior might be undesirable, but to others this behavior may be quite desirable. To be honest, sanitizing can be quite complex as their are many ways to game the system, if you will, and I would argue that sanitization is better left to such dedicated libraries that have taken it on themselves to be the experts in this area.

@waylan
Copy link
Member

waylan commented Jun 5, 2020

You can find a great summary of XSS concerns related to Markdown in this article written by the maintainer of the PHP implementation. Note that the only way to ensure safety is to run sanitized on the output of the Markdown parser. Therefore, this is out-of-scope for Markdown parsers to handle specifically.

If you are looking for precedence, GitHub documents their processes in github/Markup. Note that in step 1 the Markdown is converted to HTML, however, sanitation happens in step 2, separate from the Markdown parser (see github/markup#1246 for a, possibly out-of-date, list of what they sanitize).

In previous versions of Python-Markdown we offered a safe-mode, however, as explained in the 3.0 release notes:

The so-called “safe mode” was never actually “safe” which has resulted in many people having a false sense of security when using it. As an alternative, the developers of Python-Markdown recommend that any untrusted content be passed through an HTML sanitizer (like Bleach) after being converted to HTML by markdown. In fact, Bleach Whitelist provides a curated list of tags, attributes, and styles suitable for filtering user-provided HTML using bleach.

Ultimately, it is the user (of Python-Markdown) who knowns how the output will be used and is in the best position to assess their specific needs. Therefore, that is who is in the best position to chose what sanitation, if any, needs to be done. We will not be adding any sanitation to the library itself.

@waylan waylan closed this as completed Jun 5, 2020
@waylan waylan added the 3rd-party Should be implemented as a third party extension. label Jun 5, 2020
@chrisjohansson
Copy link
Author

Hi
Thank you for your comments. I completely understand your point about where the responsibility of this lies - the only problem is that Netbox feels the opposite and that risks putting the end users in a bind.
I also agree that html sanitising is very complex and easy to game - what I proposed was a (configurable and optional) allowlist which is quite simple and actually very hard to game (if not startstwith("https://") raise...).
Still I appreciate the feedback and will revert back to Netbox team.
/Christian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd-party Should be implemented as a third party extension.
Projects
None yet
Development

No branches or pull requests

3 participants