19

I am trying to put a general purpose function together that will sanitize input to a Mysql database. So far this is what I have:

function sanitize($input){ if(get_magic_quotes_qpc($input)){ $input = trim($input); // get rid of white space left and right $input = htmlentities($input); // convert symbols to html entities return $input; } else { $input = htmlentities($input); // convert symbols to html entities $input = addslashes($input); // server doesn't add slashes, so we will add them to escape ',",\,NULL $input = mysql_real_escape_string($input); // escapes \x00, \n, \r, \, ', " and \x1a return $input; } }

If i understood the definition of get_magic_quotes_qpc(). This is set by the php server to automatically escape characters instead of needing to use addslashes().

Have I used addslashes() and mysql_real_escape_string() correctly together and is there anything else I could add to increase the sanitization.

Thanks

Improve this question
1
  • 2
    Combining as many escaping functions as possible is not a good approach. You'll just end up with triple escaped data, and can't use it for anything but in-html output. Commented Feb 4, 2012 at 20:56

5 Answers 5

Reset to default
34

htmlentities() is unnecessary to make data safe for SQL. It's used when echoing data values to HTML output, to avoid XSS vulnerabilities. That's also an important security issue you need to be mindful of, but it's not related to SQL.

addslashes() is redundant with mysql_real_escape_string. You'll end up with literal backslashes in your strings in the database.

Don't use magic quotes. This feature has been deprecated for many years. Don't deploy PHP code to an environment where magic quotes is enabled. If it's enabled, turn it off. If it's a hosted environment and they won't turn off magic quotes, get a new hosting provider.

Don't use ext/mysql. It doesn't support query parameters, transactions, or OO usage.

Update: ext/mysql was deprecated in PHP 5.5.0 (2013-06-20), and removed in PHP 7.0.0 (2015-12-03). You really can't use it.

Use PDO, and make your queries safer by using prepared queries.

For more details about writing safe SQL, read my presentation SQL Injection Myths and Fallacies.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

6

Magic quotes are deprecated. Turn them off if you can :).

The second part addslashes and mysql_real_escape_String does pretty much the same (similar) thing. Just try 

addslashes( '\\') // and mysql_real_escape_string( '\\')

Result should be \\ so if you use

 mysql_real_escape_string( addslashes( '\\'))

you should get \\ (or '\\\\' as string). Use only mysql_real_escape_string (better) OR addslashes, never both.

I recommend to use PDO instead of raw functions and manual escaping.

Improve this answer

Comments

1

Why do you want to apply htmlentities before saving data to the database? What if you want to use the data for something else than just writing it out to a browser? For example for searching, partitioning data, using the data in other programming languages, etc...

The only thing you really want to apply is mysql_real_escape_string (or use PDO), nothing else.

I usually prefer to undo the effects of magic quotes entirely, always. Magic quotes is just cumbersome to work with and should never have been invented. Here's a snippet from the PHP manual to reverse the magic quotes:

if (get_magic_quotes_gpc()) { $process = array(&$_GET, &$_POST, &$_COOKIE, &$_REQUEST); while (list($key, $val) = each($process)) { foreach ($val as $k => $v) { unset($process[$key][$k]); if (is_array($v)) { $process[$key][stripslashes($k)] = $v; $process[] = &$process[$key][stripslashes($k)]; } else { $process[$key][stripslashes($k)] = stripslashes($v); } } } unset($process); }
Improve this answer

Comments

0

the worst part that adding slashes does not sanitize anything, no matter what function was used.
and it should not be used in the means of whatever "sanitization" at all.

slashes do not "sanitize" data. Slashes do escape string delimiters only. Thus, the only sanitization you can talk of, is escaping and and quoting.

Otherwise, if you won't put quotes around "sanitized" string, you will have no protection at all.

Improve this answer

Comments

-5

Use:

mysql_real_escape_string()

This will prevent bad data like DROP TABLE ;)

Improve this answer

1 Comment

this is extremely incompentent answer. mysql_real_escape_string() does not "prevent bad data". Yet you cannot use this query injection with mysql extension anyway

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.