The code given below is a portion of a simple login script code..

if (isset ($_POST['login'])) { 
                $username = $_POST['username']; 
                $password = $_POST['password'];
                $password_hash = sha1($password);

                $query = "SELECT id,username FROM users WHERE username = '".mysqli_real_escape_string($link,$username)."' AND password ='".$password_hash."'";

Here is some of my doubts:

  1. Is the above code vulnerable to any kinds (error,blind,union based) of SQL injection?
  2. If the above code is vulnerable to SQL injection attacks, how and why can the attacker launch the attack?
  3. If the above code is safe, why do people still go for Prepared Statements etc...?
  • 1
    This may help. codinghorror.com/blog/2005/04/…
    – ponsfonze
    Jan 27, 2013 at 0:11
  • 2
    -1 Akam Omer does not have the correct answer. You should select Bobince's answer and use parametrized quires.
    – rook
    Jan 28, 2013 at 17:48
  • Asking this question and waiting for an answer is (A) less authoritative, (B) slower and (C) more effort than just parameterizing your SQL. So why didn't you?
    – Matt
    Apr 17, 2014 at 7:41

2 Answers 2


Is the above code vulnerable to any kinds (error,blind,union based) of SQL injection?

No, but only because sha1() happens by luck to produce output that never contains any SQL metacharacters.

Your query-creating code should not need to know about that implementation detail. It should be escaping the $password_hash as well, so that the line of code works on its own, and can be verified as correct and safe without having to look at the wider context. Otherwise, when you change how that $password_hash is calculated (say, to store a more secure salted hash), you may find that you've just created a vulnerability in that query code. Creating unnecessary coupling between disparate bits of code, potentially in different files, is in general a bad idea.

If the above code is safe, why do people still go for Prepared Statements etc...?

Because prepared statements are typically easier to read, understand and verify; it is harder to accidentally omit an escape. Compare:

$query = "SELECT id,username FROM users WHERE username = '"
         .mysqli_real_escape_string($link, $username)
         ."' AND password ='"
         .mysqli_real_escape_string($link, $password_hash)


$query = $mysqli->prepare('SELECT id,username FROM users WHERE username=? AND password=?')
$query->bind_param('ss', $username, $password_hash);

Although I'm not a huge fan of mysqli's particular brand of parameterised queries (especially binding to variables instead of values), the latter still seems simpler and more maintainable to me.

  • 2
    +1 i am glad someone mentioned parametrized queries.
    – rook
    Jan 28, 2013 at 17:48
  • Escaping can be done better though: pastebin.com/tPaxi3N7. escape here additionally inspects the type and wraps in quotes if necessary (not null or a number). Just saying though, parametrization ftw.
    – Esailija
    Apr 12, 2013 at 14:57

You are using SHA-1 for your password hash without a salt even. This is bad bad bad. See How to securely hash passwords? for a much better way to hash your passwords.

Secondly, see How to prevent SQL injection in PHP for an explanation of mysqli_real_escape_string vs prepared statements.

You might forget to escape strings. Using prepared statements for your entire application makes it much less likely that you will be vulnerable to SQL injection attacks.

Plus that escape string syntax is plain ugly.

Thirdly, why are you using the WHERE clause to perform authentication instead of retrieving both the username and password hash from your database and performing a comparison?

  • 1
    Thanks for reply.. but my focus is not about securing password yet.
    – ysj
    Jan 26, 2013 at 16:08
  • I don't see anything wrong with my where clause? User enter username and password. I look into my db for records, if 1 records is found i authenticate them? As said, this is just a portion of the login script. Your answers doesn't solve all my doubts.
    – ysj
    Jan 27, 2013 at 4:51
  • 1
    @shaun - Your method is in principle susceptible to timing attacks; e.g., if the username doesn't exist the SQL query will return quicker than if the username does exist and the password doesn't match. In principle you could also figure out the prefix for the salted hash by a timing attack. (E.g., if the hash in the db starts with 2cf24 ... then a password with hash starting with 2cfaa will fail slower than one completely wrong starting with 3ab69). If you can get the first 4 characters of the sha256 pass; you can filter a list of common passwords.
    – dr jimbob
    Jan 28, 2013 at 20:56

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .