Opened 14 years ago
Closed 14 years ago
#13980 closed (invalid)
The markdown template tag in django.contrib.markup should escape preexisting HTML, as its output is marked as safe
| Reported by: | Fletcher Tomalty | Owned by: | nobody |
|---|---|---|---|
| Component: | Contrib apps | Version: | 1.2 |
| Severity: | Keywords: | markup, markdown, safe, escape | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I'm not sure about the other markup options, but Markdown itself should definitely not be marked as safe. You can easily render <script type="text/javascript">alert('pwned')</script> with Markdown, and with that is_safe = True there, the HTML will not be escaped.
Attachments (1)
Change History (4)
by , 14 years ago
comment:2 by , 14 years ago
| Keywords: | markup markdown safe escape added |
|---|---|
| Patch needs improvement: | unset |
| Summary: | The markdown template tag in django.contrib.markup should not be marked as safe output → The markdown template tag in django.contrib.markup should escape preexisting HTML, as its output is marked as safe |
You're right. That was stupid of me.
But I was also wrong about the bug/security issue. Markdown would be totally broken if we escaped its output (the user would just see HTML code), so we obviously can't remove the is_safe = True line.
What should really be done is to actually escape the input before Markdown processes it, and continue to mark it as safe output. We can do this using django.utils.html.escape.
comment:3 by , 14 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
Wow. I just realized that's what the safe mode feature is for. DISREGARD THIS.
Removed the is_safe = True so that markdown output will be escaped