Do you see any flaws in these prepared statements to avoid SQL injection?

I have used this guide to implement prepared statements in order to avoid SQL Injection.

I made some tests and no error was shown. However, I would like to ask you if you see any flaw in the code, so that I can improve it.

This is the code for a simple login:

User:

Pass:

This is Conexion_PDO.php:

prepare("SELECT * FROM user where email = ? AND
password = ?");
if ($statement->execute(array($_GET['user'],md5($_POST['pass'])))) {
while ($rs = $statement->fetch()) {
print_r($rs);
}   

$gbd = null;
} catch (PDOException $e) {
print "¡Error!: " . $e->getMessage() . "";
die();
}

?>

Your code does not have an SQL injection vulnerability because you are using placeholders instead of interpolating the variables. The way you interface with the database is laudable.

Other aspects of your code can be improved:

-
The variables $gdb, $statement, $rs, and $e are globals, which is not acceptable within a larger system. Enclose your code with a function to make them locals.

Otherwise, you could accidentally overwrite variables from other parts in your code (e.g. when you include this snippet). Consider what happens if $gdb is already assigned: You try to clean up your usage by assigning null when you are finished, but the other code now suddenly has its contents for $gdb deleted.

-
You do not seem to be using any intendation. Indent your code properly so that it can be understood easily.

-
You use the literal ¡ inside a string, and print it directly to the output. As this is a non-ASCII character, this can get problematic depending on the encoding of your source code and the character encoding in the HTTP header and the HTML document.

-
HTML markup is a kind of code as well – it too wants to be indented :)

But now something important:


Do not use md5 to hash passwords!

• MD5 is extremely weak, and an increasing number of attacks are being discovered. Do not use it for anything in new systems.

• It is meant for checksums (e.g. when comparing files), not for hashing passwords. Such a checksum is designed that a subtle change in the input changes the output of the hash function drastically. It is not designed to irreversibly scramble a secret.

• It is far to easy to calculate the original password from a given hash, e.g. using rainbow tables. You do not even use a salt to defend against this (but even with a salt, an MD5-hashed password can be cracked within reasonable time).

You should be using a key-stretching algorithm like bcrypt. I already wrote a rant about secure password hashing in another answer which I would ask you to consult for a better hashing solution.

StackExchange Code Review Q#38237, answer score: 3