Prevent Cross-site Scripting in PHP

Question

Im not sure that it is the right way but I use this functions for inputs . for example a contact form:

RemoveXSS(mysql_real_escape_string($_POST['input']))

But with scanning, there is this result:

Parameter Name: Query Based 
Parameter Type: FullQueryString 
Attack Pattern: /"ns="alert(0x00058B) 

I cant see anything in page when clicking on above url. But yet there is anything wrong in my code.

///////////

function RemoveXSS($val)
{

$val = preg_replace('/([\x00-\x08,\x0b-\x0c,\x0e-\x19])/', '', $val);

// straight replacements, the user should never need these since they're normal characters
// this prevents like <IMG SRC=&#X40&#X61&#X76&#X61&#X73&#X63&#X72&#X69&#X70&#X74&#X3A &#X61&#X6C&#X65&#X72&#X74&#X28&#X27&#X58&#X53&#X53&#X27&#X29>
$search = 'abcdefghijklmnopqrstuvwxyz';
$search .= 'ABCDEFGHIJKLMNOPQRSTUVWXYZ';
$search .= '1234567890!@#$%^&*()';
$search .= '~`";:?+/={}[]-_|\'\\';
for ($i = 0; $i < strlen($search); $i++) {
    // ;? matches the ;, which is optional
    // 0{0,7} matches any padded zeros, which are optional and go up to 8 chars

    // &#x0040 @ search for the hex values
    $val = preg_replace('/(&#[xX]0{0,8}' . dechex(ord($search[$i])) . ';?)/i', $search[$i],
        $val); // with a ;
    // &#00064 @ 0{0,7} matches '0' zero to seven times
    $val = preg_replace('/(&#0{0,8}' . ord($search[$i]) . ';?)/', $search[$i], $val); // with a ;
}

// now the only remaining whitespace attacks are \t, \n, and \r
$ra1 = array('javascript', 'vbscript', 'expression', 'applet', 'meta', 'xml',
    'blink', 'link', 'style', 'script', 'embed', 'object', 'iframe', 'frame',
    'frameset', 'ilayer', 'layer', 'bgsound', 'title', 'base');
$ra2 = array('onabort', 'onactivate', 'onafterprint', 'onafterupdate',
    'onbeforeactivate', 'onbeforecopy', 'onbeforecut', 'onbeforedeactivate',
    'onbeforeeditfocus', 'onbeforepaste', 'onbeforeprint', 'onbeforeunload',
    'onbeforeupdate', 'onblur', 'onbounce', 'oncellchange', 'onchange', 'onclick',
    'oncontextmenu', 'oncontrolselect', 'oncopy', 'oncut', 'ondataavailable',
    'ondatasetchanged', 'ondatasetcomplete', 'ondblclick', 'ondeactivate', 'ondrag',
    'ondragend', 'ondragenter', 'ondragleave', 'ondragover', 'ondragstart', 'ondrop',
    'onerror', 'onerrorupdate', 'onfilterchange', 'onfinish', 'onfocus', 'onfocusin',
    'onfocusout', 'onhelp', 'onkeydown', 'onkeypress', 'onkeyup', 'onlayoutcomplete',
    'onload', 'onlosecapture', 'onmousedown', 'onmouseenter', 'onmouseleave',
    'onmousemove', 'onmouseout', 'onmouseover', 'onmouseup', 'onmousewheel',
    'onmove', 'onmoveend', 'onmovestart', 'onpaste', 'onpropertychange',
    'onreadystatechange', 'onreset', 'onresize', 'onresizeend', 'onresizestart',
    'onrowenter', 'onrowexit', 'onrowsdelete', 'onrowsinserted', 'onscroll',
    'onselect', 'onselectionchange', 'onselectstart', 'onstart', 'onstop',
    'onsubmit', 'onunload');
$ra = array_merge($ra1, $ra2);

$found = true; // keep replacing as long as the previous round replaced something
while ($found == true) {
    $val_before = $val;
    for ($i = 0; $i < sizeof($ra); $i++) {
        $pattern = '/';
        for ($j = 0; $j < strlen($ra[$i]); $j++) {
            if ($j > 0) {
                $pattern .= '(';
                $pattern .= '(&#[xX]0{0,8}([9ab]);)';
                $pattern .= '|';
                $pattern .= '|(&#0{0,8}([9|10|13]);)';
                $pattern .= ')*';
            }
            $pattern .= $ra[$i][$j];
        }
        $pattern .= '/i';
        $replacement = substr($ra[$i], 0, 2) . '<x>' . substr($ra[$i], 2); // add in <> to nerf the tag
        $val = preg_replace($pattern, $replacement, $val); // filter out the hex tags
        if ($val_before == $val) {
            // no replacements were made, so exit the loop
            $found = false;
        }
    }
}
 return $val;
}

Thanks in advance

Answer 1

This seems like massive overkill to me. What's wrong with something like htmlspecialchars($val) or filter_var($val, FILTER_SANITIZE_STRING);

Answer 2

You're mixing up protection from XSS (injection into the browser) and SQL injection.

mysql_real_escape_string is used for escaping the string to be used in a SQL query, e.g. to avoid injection in "SELECT * FROM foo WHERE name=".$name

An example of a "bad" string would be a SQL command that would get executed.

For browser output, htmlentities() or htmlspecialchars() can be used to escape a string that is outputted from being vulnerable to XSS.

An example of a "bad" string would be: a user who registers with a name that contains HTML tags. If you output this user's name without escaping, the HTML tags will get injected.

There is no reason to manually filter for every possible JavaScript attribute or tag -- you'll likely miss some anyway. htmlentities()/htmlspecialchars() would have escaped something like

<script language="JavaScript">

with

<script language="JavaScript">

so the code would just get safely displayed by the browser instead of being executed.

The summary is: there are two types of escaping, one for SQL, one for HTML. It does not make sense to mix them.

Answer 3

Mixing sql injection and xss filters is a bad idea. They are very different attacks, and they use very different control characters.

If you just want to stop XSS use htmlspeiclchars($var,ENT_QUOTES);. If you only want to allow safe html then use HTML Purifier.

If you want to stop sql injection use parameterized queries with PDO.