PHP best practice on user authentication for a website?

Question

i built several sites with very simple code of php and so far so good, but i happens that I'm now worried about topic "security" i wonder what is the best way to create a login/authentication process in PHP

this is what I currently do:

during registration the user submit an email and a password the password will be stored, in mysql, as an md5 string, so anybody but the user knows it.

when the user login, i do

SELECT * FROM usertable WHERE email = $emailsubmitted AND pass = md5($passsubmitted)

then if the sizeof the resulting array is more then zero, it means the user exists

so I set

session_start(); 
$_SESSION['logged'] = 'true';
$_SESSION[userid] = userid;

so for everypage the user browse i'll perform a check to see if the session variable exists.

BOTTOM LINE: I wonder if this is safe enough and how it can be improved.

Answer 1

Your SQL statement has an injection vulnerability in it. (This is under the assumption that the "submitted" suffix means the variable hasn't had any filtering done on it.) If a malicious user were to submit 'admin@example.com' AND 1=1;-- as an email address, they could log in with the credentials of "admin@example.com", irrespective of the password. I'd suggest to secure the input to the SQL and/or use stored procedures. I'd also suggest loading only the columns that you absolutely need; it will improve the speed of the query and allow less state to be held in suspension outside of the database.

Additionally, implement salting on the password. If someone were to retrieve the user's data from the database, the passwords would be an easy target for bruteforce and dictionary attacks (like rainbow tables). If you're really paranoid, consider switching from MD5 to SHA or another hashing function.

Make sure of which variables are set in your php.ini file and that they are set to values you expect. Depending on those settings, the array assignment to $_SESSION is also insecure. Some old web applications utilized a PHP 'feature' which made variables in the query string become global variables in the web application, which meant that when it executed $_SESSION['userid'] = $userid;, if a malicious user attached ?userid=1 to their query string, they would become the user with the user ID of 1, which often was the first user (or administrator).

Answer 2

The general logic is ok, yes. However, A simple md5 of the password is not good.

• Not salting the password leaves the hash open to rainbow table lookups.

• md5 is generally not considered a good hashing mechanism for passwords. I encourage you to take a look at http://www.openwall.com/phpass/

Side-note: Your SQL appears to be open to SQL injection.

Answer 3

On top of the previously mentioned SQL injections I would recommend that you not check if the array is greater than zero, but instead if the array is equal to one.

The reason is that let's say somebody does modify your database and just runs a simple query to clear all of the passwords or set them to a specific entry, then checking for equal to one would stop that.

Also, let's say multiple users have the same password and they somehow SQL inject the username or you forget to check the username, then once again you will be safe with checking equal to one.

Ultimately, it is a minor thing, but when it comes to security every little bit counts.

Answer 4

The first thing i note is that you apply the MD5 hash to a php variable. That makes me think the password travels clear on the channel.

You should apply the MD5 Hash client-side with js here an example:

http://phpjs.org/functions/md5/

If you want to add another layer of security you should also consider using a salt in addition to the standard hash. This will shield you from any dictyonary attack or reverse hasing (see this: http://tools.benramsey.com/md5/)