D.W. has some excellent points, but I'd just point out a few things:
This code does four things:
- Removes characters and constructs that can trick browsers.
- Makes sure all HTML entities are well-formed.
- Makes sure all HTML tags and attributes are well-formed.
- Makes sure no HTML tags contain URLs with a disallowed protocol (e.g.
javascript:
).
Assuming it does an excellent job of all the things on this list, there are a few notable omissions : tag balancing, encoding level attacks, link spam.
Even if it does those well, HTMLPurify has been around and been attacked. I can think of at least one security academic off the top of my head, who makes sure HTMLPurify is patched and stable before publishing new attacks. If Drupal doesn't receive similar scrutiny then I'd use the more hardened one.
Tag Balancing
If your users' security depends on them being able to distinguish content that you authored from that authored by commenters or other third parties, then tag balancing is important.
Imagine that you used <table>
s for formatting. Unbalanced tags can let them sneak content out of the region that appears to be third-party content into a region of the page that appears to be controlled by the site owners. For example, if your white-list included otherwise innocuous formatting tags like <table>
then
</table>
<center>If you have any questions,
<a href="support@deceptively-similar-name.com">contact us</a>
<br>Bogus copyright</center><br><br><br><br><sub><sub><sub><sub><sub>
might let the attacker forge a footer that contains phishing links.
A similarly innocuous seeming </ul>
might help an attacker break out of a list of user comments that are displayed using <ul><li>...</ul>
per semantic HTML.
User configurable white-lists give you a lot of room to hang yourself here since HTML experts rightly assume that balanced elements like <table>
and <ul>
don't do anything security sensitive (they just create boxes around flowable content), but individual tags are problematic.
Encoding level attacks
If the attacker can get content sanitized content into the first few kB of an HTML page that does not have a Content-type
header that specifies an encoding, then it might be able to trick IE into treating the page as UTF-7, avoiding all the other sanitization.
This may fall under "constructs that can trick browsers", but the source code on that page does not indicate that it does.
Link spam
If you allow links but the sanitizer does not add rel="nofollow"
to links, then your reputation can be hijacked.