Is this code escapable or is it vulnerable to SQL Injection

Question

I'm running some tests for this login system im writing with my friend and we already had written our code with escaping, and not preparing. We're making sure it is invulnarable to anything put as a post_user and post_pass variable. Can you please check?

$_POST['post_user'] = mysql_real_escape_string($_POST['post_user']);

$_POST['post_pass'] = mysql_real_escape_string($_POST['post_pass']);

$query = mysql_num_rows(mysql_query("SELECT * FROM `users` WHERE
`user`='".$_POST['post_user']."' AND `pass`='".md5($_POST['post_pass'])."' AND
`rank`='0'"));

if($query == 1) {

$_SESSION[$this->host().'-us_user'] = $_POST['post_user'];

$_SESSION[$this->host().'-us_pass'] = md5($_POST['post_pass']);

$_SESSION[$this->host().'-us_token'] = $this->generateToken(16);

}

Answer 1

There are 2 faults with this approach, both coming from a single delusion.

mysql_real_escape_string doesn't "protect" your data. So, it should never be used for the purpose of whatever "sanitizing". Using this function like this, you are exposing yourself to two not immediate but quite possible dangers.

1. Escaping password before hashing it may spoil the resulting hash.
2. Escaping any value beside SQL strings will result in injection.

That's why you should always use parameterizing instead of "escaping". Just because parameterizing is doing its job, while "escaping" is used out of mere confusion.
I wrote a through explanation on the whole matter with escaping / parameterizing in a article you are welcome to read.

Answer 2

It should be fine since you are using mysql_real_escape_string.

If you are building a new system, you should look into using PDO or MySQLI prepared statements, it is easy and less prone to security issues.

The only issue I see is that you are storing the Password in the session, this is a very bad idea specially if you are on shared hosting.

Answer 3

$_POST['post_user'] = mysql_real_escape_string($_POST['post_user']);

$_POST['post_pass'] = mysql_real_escape_string($_POST['post_pass']);

$query = mysql_num_rows(mysql_query("SELECT * FROM `users` WHERE
  `user`='".$_POST['post_user']."' AND
  `pass`='".md5($_POST['post_pass'])."' AND
  `rank`='0'"));

Both of these escapes are wrong.

1. If you want to display the user name on a web page, it might end up with spurious backslashes. The MySQL-escaped string should only be used to pass data to MySQL.
2. The password escape may introduce backslashes into the password, which leads you to store the MD5 hash of a different password than the one entered by the user. Escape the MD5 output if you must.

You should also read the excellent article mentioned in Your Common Sense's answer.