Major HTMLawed Security Issues

bugslayer

HTMLawed is used by Vanilla to sanitize HTML, however, that is NOT what it is intended for. HTMLawed is only designed to be used to ensure valid HTML. HTMLawed fails to sanitize against a number of major attack vectors, especially in the style attribute, which can be used for click-jacking, phishing, web-page overlays, defacement, and more.

This is mentioned at http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/htmLawed_README.htm#s2.5

For example, the following snippet can be used to display a div outside the post area:

<div style="width: 100px; height: 100px; position: absolute; bottom: 0; right: 0; background: red; opacity: 0.5; z-index: 10000;">this is a security test</div>

A slight alteration could be used to exploit users via a clickjacking type attack:

<a href="http://attack.example.com" style="display: block; width: 100000px; height: 10000px; position: absolute; top: 0; left: 0; opacity: 0.0; z-index: 10000;">this is a security test</a>

You can test either of the above non-persistently by copying into a reply and clicking preview.

I strongly recommend returning to something intended for security, such as HTML Purifier. Alternately, it is supposedly possible to write your own filter to sanitize the style attribute yourself. If abandoning HTMLawed is not an option, it might be ideal to lock down all styles except those used for basic text formatting.

bobtheman

great topic. didnt v2 use html purifier at one point and time?

bobtheman

@bugslayer

it appears that htmlawed does work to purify html. the site says

"use to filter, secure & sanitize HTML in blog comments or forum posts, generate XML-compatible feed items from web-page excerpts, convert HTML to XHTML, pretty-print HTML, scrape web-pages, reduce spam, remove XSS code, etc."
http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/index.php

the plugin shows its using htmlawed 1.1.9.3 and the most recent version is 1.1.9.4.

Maybe the plugin is not configured correctly, or the older version doesn't catch clickjacking?

bobtheman

I tested your clickjacking script on htmlawed 1.1.9.4, and it still works.

you can test it for yourself here
http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/htmLawedTest.php

bobtheman

update

tested clickjacking script on html purifier and it stopped the attack

http://htmlpurifier.org/demo.php

patnaik

htmLawed can easily handle the kind of attack mentioned above. It's a matter of configuring it properly. Denying the 'style' attribute is a simple solution (set the htmLawed config. parameter 'deny_attribute' to include 'style'). If a blanket denial of 'style' is not possible, it's value can be checked using the htmLawed parameter 'spec' to conditionally deny it as needed (e.g., if in a 'style' value, a z-index is set or if display is set to block). Many have misunderstood and misspoken of htmLawed's abilities without fully reading its documentation or configuring it properly.

bugslayer

@bobtheman, the marketing jargon in this case does a great disservice to the community. The bottom line is that "safe" mode isn't really safe. It's a known issue, and handled only by briefly documenting it. I raised this concern a while back when looking into HTMLawed for other purposes, and was told "we'll make it more clear in the documentation." Unfortunately, many products (such as Vanilla) take the promises at face value, and never notice the fine print.

There is a plugin to use HTML Purifier instead, and I plan to use that as a workaround. I'm raising the issue here because a security issue like this could cripple Vanilla's reputation, which would be a rotten shame.

@patnaik, Blanket denial of style would be a big problem for Vanilla, especially for forums where WYSIWYG editors are in use. Actually processing the contents of style would be a real chore, especially since the processing would have to be cross browser secure against any type of malformed styles that people could come up with. The only safe solution I can think of would be to parse the styles and rebuild them based on a whitelist and value validation. This isn't easy.

I'm sure this seems to you like an attack on HTMLawed. I'm sorry for that. I really desperately want to be able to use it (it would be VERY convenient.) My problem is that, as currently implemented, it isn't a safe/suitable way to sanitize of this type of content. It could be patched with a custom filter, but honestly, I couldn't find any "easy" way to do this. The recommended snippet glazed over the issue of how to *actually* use CSS Tidy to do this. After digging through the CSS Tidy documentation, I don't think it can actually be done that way, because CSS Tidy doesn't appear to support any type of custom property filtering.

bobtheman

You may have a valid point

@Mark @Todd
Have you had a chance to read over this discussion? care to comment?

Todd

Curses. The problem with HtmlPurifier is that it takes a ton of memory and slows things down greatly. I'm wondering if I can deny some of the style attributes such as position an whatnot?

garymardell

Seems like we should go back to purifier and some uber caching.

bobtheman

i havent had time to look fully into this but, @Todd is the htmlawed project still actively being developed compared to html purifier ? This would be my first question

if htmlawed is still active, are our setting just not configured correctly, and this is allowing this clickjacking script? Or is this a known issue that maybe the htmlawed team is working on? Maybe we could report this and submit a fix, then update the plugin on our side.

Mark

@bobtheman - Correct me if I'm wrong, but I think @patnaik *is* one of the HtmLawed developers.

@patnaik - any chance you could help us to tighten up our htmLawed implementation so we don't have to go back to HtmlPurifier?

Todd

I just pushed a potential fix that filters out any style attributes with a position, z-index, or opacity. This is on unstable.

bugslayer

@Todd, I agree on the memory and speed issues. That's why I was looking into HTMLawed when I first learned about this issue several months ago.

I looked at your code and the change helps, but there are still lots of ways around it. I'll send you a PM with more details, since this will be outside the realm of obvious attack vectors.

Sorry to be such a pain!

glow

how can i configure htmLawed to get rid off all "style" attribute?

pixeline

idea: have those cases in which a style attribute is to be accepted (WYSIWYG editors and such) if they have a sibling attribute class="{token: }" that is a md5() of a salted value, to be specified by the Administrator via an htmlLawed option the Dashboard Settings, and have the plugin remove any instance of style that does not comply with that? That way you don't have to fork htmlLawed or rely on them fixing this specific case?

bugslayer

@pixeline, That doesn't add any security, because the javascript would have to have access to the salt, which in turn means that the attacker has access to the salt. The attacker could craft the malicious code and salt it without using the WYSIWYG editor.

Todd

@glow, I think I'm going to add a config setting to disallow all style and class attributes within the Html. People that want wysiwyg can disable this and use HtmlPurifier.

bugslayer

@Todd, it would be good to put a comment with that option indicating what the users need to do to keep their forum secure. Otherwise they're likely to open a hole without realizing it.

Todd

I just pushed a change to unstable with a Garden.Html.SafeStyles option that will disable class and style attributes entirely. It is set to true in config-defaults.php with a comment saying that its meant to prevent click-jacking attacks.

This will cause problems for people with wysiwyg and BBCode, but I think those issues can be dealt with separately.

Mark

This issue has been addressed in Vanilla 2.0.10