13

I'm trying to come up with a way to effectively easily clean all POST and GET variables with a single function. Here's the function itself:

//clean the user's input
function cleanInput($value, $link = '')
{
    //if the variable is an array, recurse into it
    if(is_array($value))
    {
        //for each element in the array...
        foreach($value as $key => $val)
        {
            //...clean the content of each variable in the array
            $value[$key] = cleanInput($val);
        }

        //return clean array
        return $value;
    }
    else
    {
        return mysql_real_escape_string(strip_tags(trim($value)), $link);
    }
}

And here's the code that would call it:

//This stops SQL Injection in POST vars
foreach ($_POST as $key => $value)
{
    $_POST[$key] = cleanInput($value, $link);
}

//This stops SQL Injection in GET vars
foreach ($_GET as $key => $value)
{
    $_GET[$key] = cleanInput($value, $link);
}

To me this seems like it should work. But for some reason it won't return arrays from some checkboxes I have in a form. They keep coming out blank.

I've tested my code without the above function and it works fine, I just want that added bit of security in there.

Thanks!

tscully
  • 151
  • 1
  • 2
  • 8
  • 1
    You should avoid trying to sanitize everything for every context. That only hinders your application and makes it harder to secure when you want to recreate lost functionality. This is a valid reason why magic_quotes was disabled. http://php.net/manual/en/security.magicquotes.php Something you're recreating in a sense here. Input needs to be sanitized for the application you are sending it to. If you are sending to the browser via HTTP, sanitize it for HTTP, and HTML. If you're sending it to SQL DB, sanitize it for SQL. – bucabay Oct 23 '09 at 00:32
  • sorry, I meant deprecated: http://php.net/manual/en/security.magicquotes.php – bucabay Oct 23 '09 at 00:33
  • Thanks for the help/advice all! It looks like I need to rethink my process. :-) – tscully Oct 23 '09 at 00:44

5 Answers5

23

Use filter_input if possible (php5 +) It keeps it a lot cleaner and as far as im aware you can sanitise and validate everything you could need using it.

You can use filter var array and for example FILTER_SANITIZE_STRING flag to filter the whole post array

filter_var_array($_POST, FILTER_SANITIZE_STRING) //just an example filter

There are loads of different filter options available on the w3schools filter reference

Marcin Szulc
  • 1,171
  • 1
  • 10
  • 21
Andrew
  • 9,967
  • 10
  • 64
  • 103
9

What you're doing isn't enough. See here.

Community
  • 1
  • 1
ryeguy
  • 65,519
  • 58
  • 198
  • 260
8

to make the recursion more elegant you could use something like array_map for example:

$_POST = array_map('mysql_real_escape_string',$_POST);

Use filter var if you can though as these kind of approaches are generally bad, just an example though ;)

robjmills
  • 18,438
  • 15
  • 77
  • 121
2

unchecked checkboxes are not sent to the server.

you may use array_walk_recursive to do what you want

w35l3y
  • 8,613
  • 3
  • 39
  • 51
2

This is the wrong way to go about cleaning input.

Applying blanket mysql escaping to absolutely everything in $_POST and $_GET is going to come back and bite you, if you still want to use the data after you've made a database query but you don't want the escape characters in there.

Use parameterised queries with mysqli or PDO and you will never need to use mysql_real_escape_string().

Ben James
  • 121,135
  • 26
  • 193
  • 155