How Can I Make This Login System More Secure

Question

I have created this php login script. I was wondering weather it was secure and if not how could I improve it.

PHP Script

<?php
include_once ("ConnectToMySql.php");

session_start();

$username = $_POST['username'];
$username = mysql_real_escape_string($username);
$password = $_POST['password'];
$password = sha1($password);

$query = "SELECT password FROM users WHERE username = '$username';";
$result = mysql_query($query);

if(mysql_num_rows($result) < 1)
{
    echo "This Username Is Not Registered!";
exit;
}
if(mysql_num_rows($result) == 1)
{
if ($password == $result)
{
    echo "Logged In!";
}

else echo "Wrong Password!";
}
?>

Thanks

Googling for "php secure login form" brings back a wealth of good posts, such as http://tinsology.net/2009/06/creating-a-secure-login-system-the-right-way/ — Joe, Feb 14 '12 at 11:09
also apply server side validations on empty values so that database query will be more secure — Sumit Neema, Feb 14 '12 at 11:18

score 4 · Answer 1 · answered Feb 14 '12 at 11:17

4

A first tip could be to show a common error for both invalid login cases: invalid username or password. That way an eventual attacker wouldn't know if the username is valid or not.

You could also make a single query matching both username and password. You would probably need more user information (to store in session?), so it would be a good idea to select those fields instead of the password (e.g. id, name).

Regarding the hashed password stored in the database, you could add a SALT to improve security. http://en.wikipedia.org/wiki/Salt_%28cryptography%29

answered Feb 14 '12 at 11:17

Stelian Matei

11,553
2
25
29

1

Yes, you should add a unique salt value for each user. You could also consider hashing on the *client* side to ensure the password is not being sent over the network (unless you're already using SSL/TLS). – adelphus Feb 14 '12 at 11:21
Your comments have been a massive help. Thank you very much! – alexjfno1 Feb 14 '12 at 12:13
I am glad they were helpful :) – Stelian Matei Feb 14 '12 at 12:17

score 1 · Answer 2 · answered Feb 14 '12 at 12:39

What I would do is change the query to the following:

"SELECT COUNT(*) FROM users WHERE username = '$username' AND password='$password';"

That way, you don't have to check if the password is correct afterwards (and you don't have to pass the sensitive data as well), you only have to check if the numbers of rows returned equal 1 and you can produce a single error message for both username/password.

How Can I Make This Login System More Secure

2 Answers2