terungwa Posted September 4, 2014 Share Posted September 4, 2014 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: allows the use of the following html tags: <h1><h2><h3><h4><h5><h6><a><br><table><ul><ol><li><p><img> remove classes, ids from html tags remove font-style, font-size, color,font-family,line-height from style tags in the text; remove javascript attributes within a tag 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. Quote Link to comment Share on other sites More sharing options...
Solution Jacques1 Posted September 4, 2014 Solution Share Posted September 4, 2014 (edited) 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 1 Quote Link to comment Share on other sites More sharing options...
terungwa Posted September 6, 2014 Author Share Posted September 6, 2014 Thank you Jacques1.I have taken a look at the htmlpurifier and it is doing exactly what i set out to achieve. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.