[closed] smarty: strip_tags modifier is not secure

[dwave]

This is something that I just reported to the guys over at Smarty's. I'm not sure it really belongs here. But its certainly a problem you should know about, too.

The strip_tags modifier is not secure and doesn't always work as expected. It is possible to get typical XSS strings past strip_tags. The risk is that users might think they could stop xss attacks just with strip_tags.

Steps to reproduce:

Code: Select all

function smarty_modifier_strip_tags($string, $replace_with_space = true)
{
    if ($replace_with_space)
        return preg_replace('!<[^>]*?>!', ' ', $string);
    else   
        return strip_tags($string);
}

$test1 = 'xss <img src="http://tctechcrunch.files.wordpress.com/2009/04/warning-sign.gif" //';

echo smarty_modifier_strip_tags($test1);

Smarty versions affected: all
CMS Made Simple versions affected: all

Discussion:
preg_replace('!<[^>]*?>!', ' ', $string); is not the same as strip_tags($string)
The preg_replace statment in this function does not filter out tags that are not closed and contain a space and a attribute. See example above.

Proposed resolution: It'd be better to rely on PHP's strip_tags alone since it has the far more refined tag stripping state machine.

[JeremyBASS]

Hello,
I'd say that since it's only on replace and the php native function

http://php.net/manual/en/function.strip-tags.php

doesn’t support the replace feature that this is a choice te use makes to not use. Also as doc'd you can use false to use the strip_tags ( string $str [, string $allowable_tags ] ).

If anything a default of true is what would be the issue, and that is for the smarty discussion boards.

Cheers -Jeremy