Is this a safe way to filter data and prevent SQL-injection and other attacks?

Question

I created two simple functions to filter inserted data before it's entered into a mysql query.

For formfields (I am also using regular expressions to check each field individually.

// Form filter
function filter($var) 
{               

    // HTML is not allowed
    $var = strip_tags(trim($var)); 

    // Check magic quotes and stripslashes
    if(get_magic_quotes_gpc())
    { 
        $var = stripslashes($var);
}

    // Not using it right now, is it recommended?
    // $var = htmlentities($var, ENT_QUOTES);

    // Escape
    $var = mysql_real_escape_string($var);

    // Return    
    return $var; 
}

Then for id's (sent in the URL) I am using this filter:

// ID filter
function idfilter($idfilter)
{
// Delete everything except numbers
$idfilter = ereg_replace("[^0-9]", "", $idfilter);

// Round numbers
$idfilter = round($idfilter);

// Test if the input is indeed a number
if(!is_numeric($idfilter) || $idfilter % 1 != 0)
{
    $idfilter = 0;
}

// Filter using the formfilter (above)
return filter($idfilter);
}

Are there suggestions to add or strip from these simple functions? And is it "safe"?

score 3 · Accepted Answer · answered Dec 07 '11 at 10:01

3

You're using deprecated function as magic_quotes and ereg_*. To prevent Sql injection you should use prepared statement (I suggest to use PDO) and to prevent XSS you should use strip_tags() as you're doing.

answered Dec 07 '11 at 10:01

Aurelio De Rosa

21,856
8
48
71

+1, but maybe using [htmlspecialchars](http://www.php.net/manual/en/function.htmlspecialchars.php) is more appropriate when you've content like HTML code that should be visible and not interpreted and of course should not be stripped. – Fabian Barney Dec 07 '11 at 10:20
As mentioned, dump `ereg_*` in favor of `preg_*` for any regex work. – Dan Lugg Dec 07 '11 at 10:37

score 2 · Answer 2 · answered Dec 07 '11 at 10:01

2

Use parameters in your queries instead of concatenating string.

Filters and cleaners are usually not safe enough.

answered Dec 07 '11 at 10:01

idstam

2,848
1
21
30

I don't really understand what you mean with your first line. – mat Dec 07 '11 at 10:08
See this: http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php – idstam Dec 07 '11 at 10:30

score 1 · Answer 3 · answered Dec 07 '11 at 10:07

1

If you are using integer ids idFilter() can be safely stripped down to

function idfilter($idfilter) {
  return (int)$idfilter;
}

As others have suggested, using parametrized queries is the right way to go though.

answered Dec 07 '11 at 10:07

code_burgar

12,025
4
35
53

Is this a safe way to filter data and prevent SQL-injection and other attacks?

3 Answers3

Linked