61

I have a lot of user inputs from $_GET and $_POST... At the moment I always write mysql_real_escape_string($_GET['var'])..

I would like to know whether you could make a function that secures, escapes and cleans the $_GET/$_POST arrays right away, so you won't have to deal with it each time you are working with user inputs and such.

I was thinking of an function, e.g cleanMe($input), and inside it, it should do mysql_real_escape_string, htmlspecialchars, strip_tags, stripslashes (I think that would be all to make it clean & secure) and then return the $input.

So is this possible? Making a function that works for all $_GET and $_POST, so you would do only this:

$_GET  = cleanMe($_GET);
$_POST = cleanMe($_POST);

So in your code later, when you work with e.g $_GET['blabla'] or $_POST['haha'] , they are secured, stripped and so on?

Tried myself a little:

function cleanMe($input) {
   $input = mysql_real_escape_string($input);
   $input = htmlspecialchars($input, ENT_IGNORE, 'utf-8');
   $input = strip_tags($input);
   $input = stripslashes($input);
   return $input;
}
Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206
Karem
  • 17,615
  • 72
  • 178
  • 278
  • 54
    `stripslashes()` *after* `mysql_real_escape_string()` ... my eyeees! – jensgram Nov 19 '10 at 10:15
  • I think data security depends to what they do and where they come from. All your treatments are not necessarily needed. – MatTheCat Nov 19 '10 at 10:15
  • 13
    Using a one-time master sanitize function is not the right solution. That's why [magic quotes](http://php.net/manual/en/security.magicquotes.php) failed. Escape (or use better solutions like prepared statements) *if and when* you need it. Otherwise, you will find you're escaping at the wrong times, and often more than once. All of those functions have very different purposes, and several (e.g. `mysql_real_escape_string` and `stripslashes`) are essentially inverses. See [What's the best method for sanitizing user input with PHP? ](http://stackoverflow.com/questions/129677) for more info. – Matthew Flaschen Nov 19 '10 at 10:16
  • 4
    Oah wow, cleanMe() doesn't stop all xss, and doesn't do anything to stop sqli because of `stripslashes()`. This is code is painful. – rook Nov 23 '10 at 18:48
  • possible duplicate of [What are the best PHP input sanitizing functions?](http://stackoverflow.com/questions/3126072/what-are-the-best-php-input-sanitizing-functions) – T.Todua Sep 25 '14 at 11:51

5 Answers5

130

The idea of a generic sanitation function is a broken concept.

There is one right sanitation method for every purpose. Running them all indiscriminately on a string will often break it - escaping a piece of HTML code for a SQL query will break it for use in a web page, and vice versa. Sanitation should be applied right before using the data:

  • before running a database query. The right sanitation method depends on the library you use; they are listed in How can I prevent SQL injection in PHP?

  • htmlspecialchars() for safe HTML output

  • preg_quote() for use in a regular expression

  • escapeshellarg() / escapeshellcmd() for use in an external command

  • etc. etc.

Using a "one size fits all" sanitation function is like using five kinds of highly toxic insecticide on a plant that can by definition only contain one kind of bug - only to find out that your plants are infested by a sixth kind, on which none of the insecticides work.

Always use that one right method, ideally straight before passing the data to the function. Never mix methods unless you need to.

Community
  • 1
  • 1
Pekka
  • 442,112
  • 142
  • 972
  • 1,088
  • 53
    We've been through 'automated sanitation' with magic_quotes. Never again. – Mchl Nov 19 '10 at 10:15
  • 4
    @Mchl +1 That's probably the number one *reason* for insecurity in PHP solutions. It basically encouraged programmer ignorance :) – jensgram Nov 19 '10 at 10:17
  • 4
    I would add that it's very worth considering using prepared statements (PDO or mysqli) instead for database queries. – Matthew Flaschen Nov 19 '10 at 10:20
  • 2
    +1 you hit the nail on the head, a vulnerability is entirely dependent on how the data is being used. – rook Nov 23 '10 at 18:46
  • Maybe I just dont see the need for B.S in an answer to a valid question. – Toby Allen Dec 15 '10 at 15:09
  • 13
    @Toby if you see fit to downvote a valid (and, by the way, accepted) answer because you don't like its choice of words, then that is your prerogative of course. But I personally think it's stupid. – Pekka Dec 15 '10 at 15:10
7

There is no point in simply passing the input through all these functions. All these functions have different meanings. Data doesn't get "cleaner" by calling more escape-functions.

If you want to store user input in MySQL you need to use only mysql_real_escape_string. It is then fully escaped to store safely in the database.

EDIT

Also note the problems that arise with using the other functions. If the client sends for instance a username to the server, and the username contains an ampersand (&), you don;t want to have called htmlentities before storing it in the database because then the username in the database will contain &.

Tomas
  • 5,067
  • 1
  • 35
  • 39
6

You're looking for filter_input_array(). However, I suggest only using that for business-style validation/sanitisation and not SQL input filtering.

For protection against SQL injection, use parametrised queries with mysqli or PDO.

Alan Pearce
  • 1,320
  • 10
  • 17
3

The problem is, something clean or secure for one use, won't be for another : cleaning for part of a path, for part of a mysql query, for html output (as html, or in javascript or in an input's value), for xml may require different things which contradicts.

But, some global things can be done. Try to use filter_input to get your user's input. And use prepared statements for your SQL queries.

Although, instead of a do-it-all function, you can create some class which manages your inputs. Something like that :

class inputManager{
  static function toHTML($field){
    $data = filter_input(INPUT_GET, $field, FILTER_SANITIZE_SPECIAL_CHARS);
    return $data;
  }
  static function toSQL($field, $dbType = 'mysql'){
    $data = filter_input(INPUT_GET, $field);
    if($dbType == 'mysql'){
      return mysql_real_escape_string($data);
    }
  }
}

With this kind of things, if you see any $_POST, $GET, $_REQUEST or $_COOKIE in your code, you know you have to change it. And if one day you have to change how you filter your inputs, just change the class you've made.

Arkh
  • 8,416
  • 40
  • 45
0

May I suggest to install "mod_security" if you're using apache and have full access to server?!
It did solve most of my problems. However don't rely in just one or two solutions, always write secure code ;)
UPDATE Found this PHP IDS (http://php-ids.org/); seems nice :)

Filipe YaBa Polido
  • 1,656
  • 1
  • 17
  • 39
  • What does mod_security have to do with input escaping, SQL injection, etc? – BoltClock Nov 19 '10 at 10:44
  • @BoltClock See the section Commonly Used Configuration Rules in [this article](http://www.thebitsource.com/infrastructure-operations/web-application/securing-apache-web-servers-modsecurity/), or see an other one by Symantec: [Web Security Appliance With Apache and mod_security](http://www.symantec.com/connect/articles/web-security-appliance-apache-and-modsecurity). – István Ujj-Mészáros Nov 19 '10 at 11:13
  • @BoltClock: it's like a bodyguard, even when you mess up, he's there to help you out ;) – Filipe YaBa Polido Nov 19 '10 at 14:40