#17837 closed Bug (fixed)
Markdown filter "safe" mode is vulnerable to e.g. 'onclick' attributes
Reported by: | Fletcher Tomalty | Owned by: | nobody |
---|---|---|---|
Component: | contrib.markup | Version: | dev |
Severity: | Release blocker | Keywords: | javascript, injection, xss, markdown |
Cc: | lemaire.adrien@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The Python markdown library used by Django has syntax like this, which is obviously a JS injection vulnerability:
{@onclick=alert(1)}paragraph
Somehow, Python-markdown's safe mode still allows this, and thus, Django does too. Since this "safe" mode is what developers are expected to use to render user input, this is a pretty significant security issue.
I have opened a ticket for Python-markdown here: https://github.com/waylan/Python-Markdown/issues/82
Newer versions of Python-markdown have an "enable_attributes" argument that you can pass, while older versions have a constant "ENABLE_ATTRIBUTES" that is declared at the module level. I'm not sure what the best way to fix this in Django directly would be.
Attachments (3)
Change History (11)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 years ago
Sorry, that was stupid of me. I should have done so, but it slipped my mind because this vulnerability is already public for the Python-Markdown library itself.
comment:3 by , 13 years ago
python-markdown has decided not to fix this (see github issue).
I'm not sure this falls in Django's scope. (Do we want to start white/blacklisting attrs?)
The question might be whether we want to use some third-party app here to assist in cleaning the markdown attrs.
comment:4 by , 13 years ago
At least the Django doc warns the users about that in https://docs.djangoproject.com/en/dev/ref/contrib/markup/:
Warning
The output of markup filters is marked “safe” and will not be escaped when rendered in a template. Always be careful to sanitize your inputs and make sure you are not leaving yourself vulnerable to cross-site scripting or other types of attacks.
We could complete this doc by referring some 3rd-part apps such as bleach maybe ?
This shouldn't be a release blocker anymore.
comment:5 by , 13 years ago
Cc: | added |
---|
comment:6 by , 13 years ago
Has patch: | set |
---|
After some discussion with Paul at the sprints, we decided to deprecate Python-Markdown<2.1
The "safe" arg to the filter in the patches just attached enables both the safe_mode=True and enable_attributes=False in tandem
Thanks for the report. In future, please send security-related bugs to security@… rather than filing them in Trac (the Trac new-ticket form makes this same request right above the form.)