Search code examples
phpmysqlsqlsql-injectionsanitization

Effective protection function against SQL injection


I found this sanitizing function in a free software:

function VerifChamps($valeur)
{
$verif = (get_magic_quotes_gpc()) ? htmlentities($valeur, ENT_QUOTES) : addslashes($valeur);
return $verif;
}

The query is then done like this:

$login=VerifChamps($_POST['name']);

mysql_select_db(..., ...);
$query = sprintf("SELECT * FROM table WHERE login='%s'", $login);

$Result = mysql_query($query, $connexion) or die(mysql_error());
$row_RsProf = mysql_fetch_assoc($Result);
mysql_free_result($Result);

How safe is this code? How is it possible to improve it to make it even more secure?

EDIT: the server is running PHP v5.2.13, with Magic Quotes turned on


Solution

  • The short answer is that it's not safe at all.

    Here's what's wrong with it...

    1. You're checking get_magic_quotes_gpc, which has been removed from PHP for years
    2. You're using htmlentities to encode the string if magic quotes is on, but not if it's off (way to corrupt your data)
    3. Why are you using htmlentities at all to send data to the database? It doesn't prevent sql injection at all.
    4. addslashes doesn't take the client connection character encoding into account when escaping your data (which makes it very unsafe)
    5. You're returning an undefined variable (i.e. NULL) making the entire function useless

    Also, mysql was deprecated and has been removed from PHP 7. Use the newer MySQLi extension instead.

    You can simply replace your entire function with the functionality provided by newer database APIs like MySQLi and PDO which offer prepared statements and parameterized queries, which are already proven to be reliable and secure. The code you're providing in your example here is clearly ancient and very insecure.