Using filter functions as intended: filter_xss and classes

Consider this code found in a page.tpl.php file.

  <body class="<?php print filter_xss($_GET['parameter']); ?>">

The documentation for filter_xss states:

Filters an HTML string to prevent cross-site-scripting (XSS) vulnerabilities.

So, you might think the above code is safe - it's using filter_xss afterall.

The problem is that filter_xss is not meant to sanitize any arbitrary sub-component of an HTML document. It has to be given complete HTML tags to work.

You can exploit the above vulnerable code with a url with ?parameter=" onload="alert('weaselpants');" which will create an alert with a nice message in it. (side note: I tend to use "monkey pants" and "weasel pants" and other oddball phrases in my debug code and alert message since they are easily recognizable as content that is not real and should get removed prior to committing code.)

Incomplete XSS fix: Using filter_xss and check_plain to filter the class

The proper way to use filter_xss, is something more like this:

<?php print filter_xss('<body class="'. $_GET['fun'] .'">', array('body')); ?>

The filter_xss function can deal with attempts to inject Javascript into a class IF it is given the entire HTML tag. However, wrapping your HTML in PHP calls isn't great style. It is likely to upset the designer and this doesn't validate that the CSS clas is valid.

So, to make the designer happier with the formatting of the HTML an idea might be to just use check_plain instead of filter_xss:

<body class="<?php print check_plain($_GET['parameter']); ?>">

This does protect against XSS, the resulting HTML of the above attack URL is:

  <body class="&quot; onload=&quot;alert(&#039;weaselpants&#039;);&quot;">

However that's not valid CSS. Heine Deelstra recently wrote about what things are and aren't valid css classes. Even if those might technically be valid they're not the kind of thing we want to see in our HTML. Let's try some further approaches.

Fixing the XSS to a known valid CSS class with a template.php function

However, probably the best way to fix this particular hunk of code would be in a template.php preprocess function. You would create a map of allowed values for the $_GET parameter, check the incoming value against those, if it is one of those then use the associated, known safe value. And if it's not an acceptable value you simply discard the data.

In your template.php:

<?php function phptemplate_preprocess_page(&$vars) {   if (isset($_GET['parameter'])) {     switch ($_GET['parameter']) {       case 'weaselpants':         $vars['body_parameter'] = 'something_funny';         break;       case 'monkeypants':         $vars['body_parameter'] = 'something_amazing';         break;       default:         $vars['body_parameter'] = '';         break;     }   } } ?>

OR, another potential way to write that is with an array of acceptable values.

<?php function phptemplate_preprocess_page(&$vars) {   $body_classes = array (     'weaselpants' => 'something_funny',     'monkeypants' => 'something_amazing',   );   $vars['body_parameter'] = '';   if (isset($_GET['parameter']) && $body_classes[$_GET['parameter']]) {     $vars['body_parameter'] = $body_classes[$_GET['parameter']];   } } ?>

Then, in your page.tpl.php:

<body class="<?php print $body_parameter; ?>">

In this case it's safe to print $body_parameter directly because you are only putting known safe values into it. One benefit of doing this process of mapping user values to known-good values is that you can be certain you will have valid css class values.

General Security Philosophies

There are two main security philosophies at play here:

Never trust user input.
You can take user input, check it against a reference list and use resulting, reliable values to print the actual data.

And of course the third security philosophy is that it's important you actually know the way your tools work.

Tags: filter_xss xss Drupal PlanetRelated Drupal Version: All versions

Original Article:

Using filter functions as intended: filter_xss and classes