What else should I be doing to sanitize user input?

Question

Recently, I had an audit run on some of my sites by a client. One of the things they came back with was that I could be sanitizing the input data a little better as people could still cause potential harm to the database.

The function below is what I am currently using (a leftover from the old developer) but I cannot see where the potential issue may lie.

The string that gets passed through to the database will be displayed via XML which in turn is read by a Flash application.

Could anyone tell me what I might be missing? Thanks

function secure_string($string)
{ 
 return (strip_tags(addslashes(mysql_real_escape_string(
                      stripslashes($string)))));
}

Answer 1

It looks like there's too much going on in that function. mysql_real_escape_string() already escapes everything you need to escape, so there's no need to run addslashes() on that. In fact, it could do more harm than good by escaping the backslashes mysql_real_escape_string() creates.

Answer 2

Depends on where the "secured" string will be used. If it's going to be used in the database, you only need mysql_real_escape_string(), nothing more. If it's going to be displayed in html, you only need htmlentities(), nothing more. In short: your code is doing way too much, which could even be harmful.

If you want to store it in the database for displaying it in html lateron (like a comment, for example), you should be using mysql_real_escape_string() when storing the string and htmlentities() when displaying it.

Answer 3

mysql_real_escape_string is the last step, you shouldn't use it your application logic. It's sole purpose is to pass strings to the database, so use it only when constructing queries. You can pass anything to mysql_real_escape_string and it will make sure you can safely store it in the database.

For the rest, it depends what do you want. If you want to strip tags, use strip_tags, etc.

Answer 4

"santizing input" is a nonsense. Input data cannot be "dirty" as such, the meaning of characters depends on the device you're sending them to and this is where you should "sanitize" the data (NB: not only user input, all data). For example, a newline symbol is completely harmless when outputting to browser, but can lead to injection if sent per email. Therefore, don't waste your time checking input for "invalid" characters, instead, think about output escaping (html, email, sql statements, shell commands etc).

// edit

because i obviously did a bad job explaining myself, here's an illustration (for the benefit of those who happen to google this topic out)

 Fig.1. Data flow in a typical web script.

 A--------------------------B------------C--------------D------E

 -------------------------         ------------------
 :  user input           :-------> :                :-------> http browser
 -------------------------         :                :-------> database
                                   : your script    :-------> mail server
 -------------------------         :                :-------> file
 :  other data sources   :-------> :                :-------> shell
 -------------------------         ------------------

we obtain data from user and/or external source (A), do something with the data (C) and send it to an extenal facility (E). Some people insist that "sanitizing" should happen at step B (receiving data), my point is that should happen on step D (sending data).

Why? Every facility has its own escaping rules, and on step B you normally don't know which one you're going to use. So which rules should you take?

If you arbitrary choose rules for X (say, database), you will run into big problems when trying to send data to Y instead (say, browser). The classical example of this wrong approach is notorious "magic_quotes". If you ever seen slashed quotes in your html forms, you probably understand what i'm talking about.

Answer 5

Better use the new PHP function filter_var() for cleaning input. New and better.

Answer 6

If your server uses php 5.2 or better, you should use filter_var for the XML part.

$output = filter_var($input, FILTER_SANITIZE_STRING);

To store something into your database, use PDO and parameterized queries.

Answer 7

It's a misnomer to try and fix the problem at input time, since the problem happens at output time. See my answer over here:

What’s the best method for sanitizing user input with PHP?