Take the 2-minute tour ×

Considering a project where will work more than one developer and that will receive constant update and maintenance, what of the two codes below can be considered the best practice in PHP, considering Readability and Security? If we talk in performace, the second option will probably be a little better, but there are ways to solve this point.

Option 1

$user = $_POST['user'];
$pass = $_POST['pass'];

// Prevent SQL Injection and XSS
$user = anti_injection($user);
$pass = anti_injection($pass);

if(strlen($user) <= 8 && strlen($pass) <= 12)
    // SQL query
    $sql = "SELECT id 
            FROM users 
            WHERE username = '$user' AND password = '$pass';";

Option 2

// Retrieve POST variables and prevent SQL Injection and XSS
$_POST['user'] = anti_injection($_POST['user']);
$_POST['pass'] = anti_injection($_POST['pass']);

if(strlen($_POST['user']) <= 8 && strlen($_POST['pass']) <= 12)
    // SQL query
    $sql = "SELECT id 
            FROM users 
            WHERE username = '" . $_POST['user']. "' AND password = '" . $_POST['pass'] . "';";


I am not using MySQL, my database is PostgreSQL

share|improve this question
If you're interested in security, why aren't you using bound parameters? –  andrewsi Jul 13 '12 at 14:46
The two are practically the same but look ugly to me but i'm a crazy coder, i abstract and do stuff a little too over the edge some time... –  Mathieu Dumoulin Jul 13 '12 at 14:49
I would always use the first option if only to ensure that I never forget to call the anti_injection thing on the values. The second option somehow magically infers that it is happening, but you can never be sure. Also, I would have been sure that the first is faster as array access is probably slower than accessing a variable – are you sure about the performance? –  poke Jul 13 '12 at 14:50
@poke when compiled to opcode, array access and variable access shouldnt be significantly different, unless you do a 1 billion loop to access 1 billion elements. –  Mathieu Dumoulin Jul 13 '12 at 14:53
Upvoted because you are taking the time to explore security for your code. (But that being said don't use either) –  Mahn Jul 13 '12 at 14:58

5 Answers 5

up vote 5 down vote accepted

Don't do either.

I can only assume anti_injection is some sort of custom filtering function, which is a Bad Idea™. If you really want to adopt either of these idea, you should be using mysql_real_escape_string.

The only way to remain secure when writing SQL queries is to use parameters, e.g. through MySQLi. The mysql_* functions are becoming deprecated anyway, so you're best to move across as soon as possible.

In fact mysql_real_escape_string is not a foolproof defense against injection attacks. Consider an integer comparison in a query, such as WHERE $var > 30. I could inject 1=1 or 100 into $var successfully, and completely break the logic.

Parameters completely separate data from query language, completely mitigating the injection risk. The server receives a query containing parameter notation, and a set of values to insert, so it can handle the query language and data completely differently.

Furthermore, you seem to be storing passwords in plaintext. This is a bad idea. You should look into a strong password storage hash algorithm such as bcrypt, which makes it very difficult to obtain plaintext passwords from the hashes.

MD5 and SHA1 are not ideal for password storage, because they are designed to be fast, meaning an attacker can quickly crack even strong salted passwords. Modern GPUs can achieve 5 billion MD5 hashes per second. In fact, there are people with dedicated hash cracking rigs, some of which can crack MD5 at 45 billion hashes per second.

You should also take a look at these awesome questions, which completely cover SQL injection attacks, password storage, and a multitude of other security issues:

Update: You mentioned you're using postgres. You can use PDO to run parameterised queries from PHP, as described briefly here.

share|improve this answer
Thanks for your tips, they will be very useful to me. I will see all questions you suggested –  Marcio Simao Jul 13 '12 at 15:23

Neither of these options is a best practice, if only for the dubious anti_injection which is almost certainly not working as advertised. There's also the matters of:

  • munging your input data before validating them
  • storing plaintext passwords
  • constructing SQL queries manually instead of using bound parameters

Depending on the scope of the project the last one might be acceptable.

Regarding performance, the first option is going to be theoretically faster because it does less array indexing. But the difference would definitely be so small as to not be observable at all. The first option is also more readable and provides better abstraction, all for just a negligible amount of extra memory.

share|improve this answer

Both are wrong. You appear to be storing the password as plain text, which is just asking for trouble.

Also, if my username is "123456", I would be unable to log in because you would escape it to \"123456\" and that would fail the "length <= 8" check.

share|improve this answer

I would suggest using PDO if you want security and performance. You cannot sql inject when using parameters. Below is an example, you need to "define" the mysql params for the below to work.

This also allows for the database to cache your query, since it won't change every time you execute with a different parameter which will increase performance as well.

:p_user and :p_pass

is used to denote the parameters and

array ( ':p_user' => $user, ':p_pass' => $pass ' )

sets the parameters to the values you need to pass in.

You should also consider adding a password salt, and storing the sha1 of that in the databased, so that if your database is compromised, your passwords are not clearly revealed to the hacker.

class users{ 
  function __construct() {
    define('MySQLHost', 'localhost');
    define('MySQLName', 'databasename');
    define('MySQLUser', 'username');
    define('MySQLPass', 'password');
    define('pwsalt', 'tScgpBOoRL7A48TzpBGUgpKINc69B4Ylpvc5Xc6k'); //random characters

  static function GetQuery($query, $params)
    $db = new PDO('pgsql:host='.MySQLHost.';dbname='.MySQLName.';', MySQLUser, MySQLPass);
    $cmd = $db->prepare($query);

      return null;
    return $cmd->fetchAll();

  static function GetUser($user, $pass)
    $query = "select id
      from `users`
      where username = :p_user and password = :p_pass";

    $rows = users::GetQuery($query, array(
      ':p_user' => $user,
      ':p_pass' => sha1($pass.pwsalt) //Append the salt to the password, hash it

    return $rows;

$user = $_POST['user'];
$pass = $_POST['pass'];
  $result = users::GetUser($user, $pass);
  if($result != null)
    print 'Login Found';
share|improve this answer
Nice example, it will be useful to me. I will read more detais in PHP docs –  Marcio Simao Jul 13 '12 at 15:23
PDO is the right and only answer to this question. –  Andy Ray Jul 13 '12 at 16:42

While I agree with the other posters, don't try to "reinvent the wheel" regarding SQL security, the question seems to be about performance and how to use the superglobals.

As best practice, do NOT mutate the superglobals ($_GET, $_POST, $_REQUEST, $_SYSTEM, etc.) I can't think of a single example where violating this rule would improve your performance, and any modification will cause uncertainty and confusion down the road.

So in this case, neither option is correct. Option1 copies a variable needlessly (a no-no according to the Google performance docs). Option 2 mutates the superglobals, which violates the maxim above. Instead do something like:

$user = anti_injection( $_POST['user'] );
$pass = anti_injection( $_POST['pass'] );

if( strlen($user) <= 8 && strlen($pass) <= 12 ) ...

However I should reiterate that "homemade sanitation" is a frightening prospect, the other commentors have elaborated quite thoroughly on this point.

share|improve this answer

Your Answer


