PHP login. Is this secure enough?

I have a project to create a simple login page. I think it is done but was wondering if anyone could offer some advice on my code? Is it secure enough and would you have any suggestions to make it better?

The login form only asks for username and password which are stored in the database.

<?php error_reporting(E_ALL); ini_set('display_errors', 1); session_start(); $error=''; // Variable To Store Error Message if($_SERVER['REQUEST_METHOD'] == 'POST') { $username = $_POST['username']; $password = $_POST['password']; $hashed_password = password_hash($password, PASSWORD_DEFAULT); include('dbconx.php');

$stmt = $con->prepare(‘SELECT * FROM admin WHERE username = ? AND password = ?’);
$stmt->bind_param(‘ss’, $username, $password);
$stmt->execute();
$stmt->bind_result($id, $username, $password);

if ($stmt->fetch()) {
if(password_verify($password, $hashed_password)) {
$_SESSION[‘login_user’] = $username;
header(“Location: confirm.php”);
exit;
}
}

else {
$error = “Username or Password is incorrect”;
}
mysqli_close($con);
}
?>

Is it secure? Mostly.
Will it work? …no.

The hashed password should be in the database. Your login should not hash a password, your signup should.

The proper workflow should be:

User enters username and pwd.
Pull the hash for that user from the database.
If there is no result row for the query: The username was invalid. (DO NOT THROW THIS AS A SPECIFIC ERROR. Keep the error generic between bad username/password.)
If there is: Verify the password, passing the user’s input and the hashed password retrieved from the database.
Login if valid.

password_hash will give you a random salt every time - so hashing the user’s login input will NOT give you the same string every time, meaning you cant query the database for a username and password combination. (If you could, you wouldnt need to verify the password - it already matched during the DB query)

2 Likes

Storing raw passwords in the database is one of the worse things to do. So it’s not at all secure.

OP isn’t storing passwords as plain text. The logic is just wrong.

The code suggests otherwise but I suppose you know better from another thread.

Also since it has not been mentioned so far I would suggest considering front-end security through your form. Things such as generating a unique token upon landing at the login page, which gets stored in a hidden field and in the session and then validating that the values are the same upon submission. Also honey pots and potentially too checking for the time elapsed between landing at the page and submitting the form… These things prevent against bots trying to brute force your login…

Hope it helps

2 Likes

No. Because if you look at

The OP isn’t storing passwords as plain text at all. The logic is just wrong because grabbing the username and password is going to be a problem. It has to only be the username.

Hashed password.

1 Like

Their is no option to sign up. The passwords are already stored in the database and the code seems to work.

If that code works as written, complain to your teacher that they are storing plaintext passwords and expecting students to produce secure scripts from insecure practices.

1 Like

If I’m reading the logic correctly, it looks like the raw unhashed passwords are being stored in the database (This is insecure!), but then the password_verify function is being used “back to front” where by the user input is hashed, then compared against the raw password in the database.

So in effect the system is no more secure than the original version where raw password was compared to raw password.

This is the root of the problem, you are not working in a realistic environment. A system that stores unhashed passwords cannot be secure.
All passwords should be hashed before recording in the database.
When you have that you can check it with something along these lines:-

$stmt = $con->prepare('SELECT password AS hash FROM admin WHERE username = ?');

You may want to select other columns such as ID or whatever yo need, but the password is the one we need specifically to validate the login.
Once you have the hashed password from the database you check it like so:-

if(password_verify($userInputPassword, $hash)) {
     // login is good
}
else{
     // Login is bad !!
}
3 Likes

That line does suggest that password hashing is used, but when it comes to the query parameters

$stmt->bind_param('ss', $username, $password); 

they are not using the hashed version.

On that, though, what’s the difference (presuming the system is updated to store hashed passwords) between the method suggested by @sama74 above where only the name is used to retrieve the details, and passing in the hashed version of the password to the query? I’ve seen a lot of people suggest the former, but wondered why that is better than hashing the user-supplied password to pass into the query.

The OP says the script “works” as it is.

The only way I can see that it would work is if the passwords in the DB are unhashed.
If they were hashed the verify function would fail every time.

That must be the case, yes.

This:-

1 Like

That line doesn’t prove anything. s stands for string. ss stands for string, string. What is the data type of a hashed password?

  • It can’t be an int (i) because the hashed password contains special characters, letters, and numbers.
  • It can’t be a double (d) because the hashed password contains special characters, letters, and numbers.
  • It can’t be a blob (b) although you could store it as a blob, but it would be stupid to.

So what else could the data type of a hashed password be?

I have attached a link to the data type that bind_param uses at the bottom. Scroll down a little bit and you will see all the data type that belongs to bind_param.

http://php.net/manual/en/mysqli-stmt.bind-param.php

Sorry, I just meant that they were passing $password, the users typed response from the form, not the hashed string they had created.

1 Like

Ah, I am starting to think that the OP is storing the passwords as plain text too. But at the same time, I am still thinking that they are storing it as hashed passwords. The reason why is because

Still doesn’t convince me. It is very vague and could mean anything. It could mean “the parameterized statements works because I took out the password column that needs to be passed”. It could mean “the code seems to work because a harry dog came into the room”. I mean it could mean anything you want it to. The only confirmation that we could get is if the OP pastes the passwords from the the password column into this thread.

While I still stand by this statement, I will correct myself.

Because the OP is putting the raw password into the query, the query would only return a result if the stored passwords were unhashed, so they would never get as far as “using” the verify function.
But supposing they did, the function would always return false in this script.

So there is no way this script could do anything that could be described as “working” if the stored passwords were hashed.
So I’m saying they are not hashed.

1 Like

Sorry, on my question above - I skim-read, and hadn’t noticed that the verification is done with password_verify(). I hastily read that as retrieving the hash and using password_hash() on the user input, and comparing the two, hence I couldn’t see why a query wouldn’t work. Obvious of course now I’ve read it properly.

1 Like