Sanitizing array values before mysql inserts

I have the following code blocks in a PHP form-handler:

function filter($data) {
    $data = trim(htmlentities(strip_tags($data)));

    if (get_magic_quotes_gpc()) {
        $data = stripslashes($data);
    }
    $data = mysql_real_escape_string($data);

    return $data;
}

foreach($_POST as $key => $value) {
    $data[$key] = filter($value);
}

I am modifying my form to now include checkbox groups:

eg:

<input type="checkbox" name="phone_prefs[]" value="prefer_home">
<input type="checkbox" name="phone_prefs[]" value="prefer_cell">
<input type="checkbox" name="phone_prefs[]" value="prefer_work">
etc.

Because of this code I now have arrays in my _POST variables rather than just strings.

Am I correct in thinking that my filter() function the will not actually sanitize arrays properly? What changes do I need to make to my filter() function to make sure the arrays for the checkboxes are sanitized completely and not an easy target for SQL injection atta开发者_C百科cks?

As for the sql injection, I would switch to PDO using a prepared statement.

You can use a simple is_array() on your values to check for an array and then loop through it. You are correct, as it is, your filter function will not handle arrays correctly.

Edit: If you use PDO and a prepared statement, you don´t need mysql_real_escape_string anymore. strip_tags, htmlentities and trim are also not needed to store the information safely in a database, they are needed when you output information to the browser (trim not of course...), although htmlspecialchars would be sufficient for that. It´s always better to prepare your information / output correctly for the medium you are outputting to at that moment.

Your function is pretty good, but if you make it recursive it'll crawl nested arrays for you

function filter(&$array) {
    $clean = array();
    foreach($array as $key => &$value ) {
        if( is_array($value) ) {
            filter($value);
        } else {
            $value = trim(strip_tags($value));
            if (get_magic_quotes_gpc()) {
                $data = stripslashes($value);
            }
            $data = mysql_real_escape_string($value);
        }
    }
}

filter($_POST); # filters $_POST and any nested arrays by reference

Edit: Leave out htmlentities(). If you need it, then use it when outputting the values - not when getting them as input.

array_walk_recursive($array,function(&$item){
     $item=mysql_real_escape_string($item);
});  

You're using a foreach on the $_POST which only loops once, using the Array and handling it like a string.

Try using:

foreach($_POST['phone_prefs'] as $key => $value)

EDIT:

I believe I misunderstood your question:

foreach($_POST as $key => $value)
    if (is_array($value))
        foreach($_POST[$key] as $key2 => $value2)
            /* Setting stuff */
    else /* Setting same stuff */

Instead of sanitizing the input manually, use always prepared statements with placeholders. That will transparently pass the input to the database in such a way that it does not need to be escaped and thus is not vulnerable to SQL injection. This is the best current practice.

See the following for more information: http://php.net/manual/en/pdo.prepared-statements.php

I use this on various sites I have created:

public function clean($dirty) {
    if (!is_array($dirty)) {
        $dirty  = ereg_replace("[\'\")(;|`,<>]", "", $dirty);
        $dirty  = mysql_real_escape_string(trim($dirty));
        $clean  = stripslashes($dirty);
        return $clean;
    }
    $clean      = array();
    foreach ($dirty as $p => $data) {
        $data   = ereg_replace("[\'\")(;|`,<>]", "", $data);
        $data   = mysql_real_escape_string(trim($data));
        $data   = stripslashes($data);
        $clean[$p] = $data;
    }
    return $clean;
}

Using mysql_real_escape_string means a MySQL connection is required before using the function, which is not best convenient. I used to use a work around in that case :

function real_escape_string($aQuery) {
    if (!is_string($aQuery)) {
        return FALSE;
    } else {
        return strtr($aQuery, array( "\x00" => '\x00', "\n" => '\n', "\r" => '\r', '\\' => '\\\\', "'" => "\'", '"' => '\"', "\x1a" => '\x1a' ));
    }
}

But definitely, the best is to use PDO prepared statement instead of mysql. You will enjoy it.