XSS flaw using MarkSmarty without HTML filter
bdurbin - November 2, 2007 - 22:31
| Project: | Markdown with SmartyPants |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Description
The Profile module filters all custom multi-line profile fields with check_markup on save. If Marksmarty is the default input format for the site, tags are allowed in multi-line text fields in a user's profile.
Steps to recreate:
Drupal 5.2
As admin, do the following:
enable Marksmarty as default input format at /admin/settings/filters
goto /admin/user/profile
add a multi-line text field item, call it 'script'
edit a user profile, adding the following text in the "script" profile field: alert('hello');
save the user profile
view the user profile
#1
I'm able to reproduce this with profile text fields, node bodies and comments - anything filtered by MarkSmarty. MarkSmarty doesn't have to be the default input format, just one available to an untrusted user.
However, it's possible for the admin to resolve this issue as follows:
These instructions assume there's no way for MarkDown to generate malicious code (via some escaping tricks for example). If you're not happy with that assumption, give MarkDown a higher weight than HTML filter, and be prepared for the loss of a certain amount of MarkDown functionality.
So, while this is a security hole, the fix might well be in the docs and perhaps something in hook_requirements in the module itself.
The INSTALL.txt does finish with:
"While no tag that Markdown creates is destructive (you can't
create objects or scripts) it will still create a few that you may not want
in comments. In this case, it is recommended that you add HTML filtering into
your input module after Marksmarty to remove trouble tags--for example,
you may not want people to use H1s in their posts."
My view is that this is not high profile enough and does not cover all cases (i.e. focuses on comments).
#2
Thinking a little longer on this...
With the number of different filters out there, maybe that hook_requirements functionality should actually be in Drupal core. When on the admin screens it would be nice to know if it's possible for a user to enter "unsafe" markup. Here, "unsafe" would be defined something like:
hook_filter_xss_safe