Marching New Lines in PHP regex

[terungwa]

While using the PHP strip_tags() function,I used  the optional second parameter to specify tags which should not be stripped. 

However with these allowable_tags, I realised strip-tags is not safe.
for example with a string such as this ($str= "<p onmouseover=\"evilscript url\"> Hi, this is an interesting link. </p>") I was able to load a page containing a script injected into it, that script was able to rip off my session cookie ID!!!.

To further strengthen data sanitization using the strip_tags() function, I have come up with this function below which does the following:

1. allows the use of the following html tags: <h1><h2><h3><h4><h5><h6><a><br><table><ul><ol><li><p><img>
2. remove classes, ids from html tags
3. remove font-style, font-size, color,font-family,line-height from style tags in the text;
4. remove javascript attributes within a tag
5. remove empty style tags

function CleanUp($InputString)
{
	
    $RemoveAttrib = "'\\s(class|id|javascript:|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup)=\"(.*?)\"'i"; //remove classes, ids and disallow these attributes/prefix within a tag;
    $InputString = strip_tags($InputString, '<h1><h2><h3><h4><h5><h6><a><br><table><ul><ol><li><p><img>');
    $InputString = preg_replace($RemoveAttrib, '', $InputString);
    $RemoveAttrib = "/(font\-size|color|font\-family|line\-height):\\s".
             "(\\d+(\\x2E\\d+\\w+|\\W)|\\w+)(;|)(\\s|)/i";
	//remove font-style, font-size, color,font-family,line-height from style tags in the text;
    //$InputString = stripslashes($tagSource);
    $InputString = preg_replace($RemoveAttrib, '', $InputString);
    $InputString = str_replace(" style=\"\"", '', $InputString); //remove empty style tags, after the preg_replace above (style="");
       return $InputString;
}

This worked well for single line text, but if I had hard returns in the text the function could not find the other tags to remove, and therefore failed. See below.

<p id=
"mike" style="line-height: 150%; class="lead">The function did not strip off the paragragh ID attribute.</p>

Or

<p
 id ="mike" style="line-height: 150%; class="lead">The function did not strip off the paragragh ID attribute.</p>

I tried marching New lines as shown below but it did not work:

 $RemoveAttrib = "'\\s(class|id|javascript:|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup)(\r\n)*?=\"(\r\n)*?(.*?)\"'i"; 

 I need help to fix this.

 

Thanks.

[Jacques1]

No, no, no.

 

Blacklisting does not work. No matter how smart you think you are, there will always be somebody who is smarter than you and knows how to circumvent your filter. This has happened to thousands of programmers before you, and it will happen to you as well. It's an arms race you cannot win: While you keep adding new filter rules, attackers keep inventing new techniques.

 

Actually, I've already broken your filter:

echo CleanUp('<img src="#" onerror="alert(\'XSS\')">');

And if you fix that, I already know a different technique. And if you fix that, I already know a different technique etc. It's hopeless.

 

XSS filtering like you do it is fundamentally wrong. If you want to allow a certain subset of HTML, then you need a professional library like HTML purifier and a whilelist(!) of acceptable elements and attributes. So instead of trying to enumerate every single attack technique out there, you specify what the user can do.

Edited September 4, 2014 by Jacques1

[terungwa]

Thank you Jacques1.I have taken a look at the htmlpurifier and it is doing exactly what i set out to achieve.