1

Topic: Allowing 'style' and clickjacking

The following shows how someone could post a comment on an htmLawed protected site, such that the comment would hijack all clicks on the site:

<a href="http://example.com/attack.html" style="display: block; z-index: 100000; opacity: 0.5; position: fixed; top: 0px; left: 0; width: 1000000px; height: 100000px; background-color: red;">&nbsp;</a>

The above code has opacity 0.5 for demonstration purposes, but imagine it with opacity 0, and you'll see the problem. I couldn't find any setting (other than denying styles completely) that would prevent this problem. It is important to be able to deny specific style attributes.

2

Re: Allowing 'style' and clickjacking

Thank you for pointing this out for others to note that permitting the 'style' attribute is dangerous. Except for URLs and a few other things, htmLawed currently does not check every CSS style property. It does provide ways for the code-developer implementing htmLawed to do such checks through the htmLawed's '$spec' argument, and through the 'hook_tag' parameter (on this and more see the documentation)

3

Re: Allowing 'style' and clickjacking

Yeah, I found that documentation after posting. I guess the big deal here for me is that attacks like this are possible with the default "safe" configuration, which is a fair concern. I imagine most folks just assume that safe means safe, when in fact their sites are still wide open (it only took a moment to find this, and a moment longer to find the issue in my other post.)

As far as fixing this with hooks, I couldn't see how to use CSS Tidy to sanitize the styles. Did I miss something?

4

Re: Allowing 'style' and clickjacking

You are right -- the 'safe' configuration does not secure this issue, and many people are unlikely to go through what exactly 'safe' implies to be aware of this. I will alter the documentation to make this clearer.

htmLawed does not implement CSS Tidy. Following is a suggested but not tested code snippet on how that might be used through 'hook_tag':

// Include CSS Tidy and define a function to use it
include_once('.../class.css.tidy.php');
function css_tidy($element, $attribute_array=0){

  // if second argument is not received, it means a closing tag is being handled
  if(is_numeric($attribute_array)){
    return "</$element>";
  } 

  if(isset($attribute_array['style'])){
    // Do the CSS Tidy filtering of 'style' value
    $value = $attribute_array['style'];
    ... // use appropriate code on $value for CSS Tidy
    // Assign 'style' the filtered value
    $attribute_array['style'] = $value;
  }

  // Build the attributes string
  $attributes = '';
  foreach($attribute_array as $k=>$v){
    $attributes .= " {$k}=\"{$v}\"";
  }

  // Return the opening tag with attributes
  static $empty_elements = array('area'=>1, 'br'=>1, 'col'=>1, 'embed'=>1, 'hr'=>1, 'img'=>1, 'input'=>1, 'isindex'=>1, 'param'=>1);
  return "<{$element}{$attributes}". (isset($empty_elements[$element]) ? ' /' : ''). '>';
}

// htmLawed config. points to above function
$config = array( ..., 'hook_tag'=>'css_tidy' , ...);

// htmLawed filtering
$out = htmLawed($in, $config...);

5

Re: Allowing 'style' and clickjacking

The documentation in the new release of htmLawed expands on the risks of site defacement, etc., if 'style' is permitted even when 'safe' is set to 1.

'safe' was or is not meant for complete adherence to site-owner's policies. It is really for security, in the implied meaning of the word.