Want to protect your cyber security and still get fast solutions? Ask a secure question today.Go Premium

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 140
  • Last Modified:

password_verify with prepared statements

I originally used mysqli object oriented way to log users in but am moving over to prepared statements. My problem is that with this code, as long as the email address is in the database, any password I type in works which is obviously terrible! I can just type "ashflajsf" as my password and it lets me in provided the email address is in the database.

$stmt = $link->prepare("SELECT `user_email`, `safe_user_id`, `user_password` FROM `db_users` WHERE `user_email` = ? AND `user_active` = ? ");
		$stmt->bind_param("ss", $email, $user_active);
		$email = clean_user_input($_POST['email']);
		$user_active = 1;
		$stmt->execute();
		$result = $stmt->get_result();
		$numRows = $result->num_rows;
		if($numRows === 1 ) {
			while($row = $result->fetch_assoc()){
			$db_password = $row['user_password'];
			if(password_verify($_POST['password'], $db_password));
			echo "user authenticated";
			} 
				
		} else {
			
			echo "Incorrect login details";
		}

Open in new window

0
Black Sulfur
Asked:
Black Sulfur
  • 5
  • 3
  • 2
1 Solution
 
Dave BaldwinFixer of ProblemsCommented:
Since you are not checking the password in the WHERE part of the SQL statement, that's not surprising.  Maybe like this?
$email = clean_user_input($_POST['email']);
$password = clean_user_input($_POST['password']);
$user_active = 1;
$stmt = $link->prepare("SELECT `user_email`, `safe_user_id`, `user_password` FROM `db_users` WHERE `user_email` = ? AND `user_active` = ? AND `user_password` = ? ");
$stmt->bind_param("ss", $email, $user_active, $password);
$stmt->execute();

Open in new window

0
 
Julian HansenCommented:
if(password_verify($_POST['password'], $db_password));

Open in new window

Take the ; of the end off the above statement.
Your echo is firing because you are terminating the if with a ;
0
 
Black SulfurAuthor Commented:
Actually, I fixed it like this

$stmt = $link->prepare("SELECT `user_email`, `safe_user_id`, `user_password` FROM `db_users` WHERE `user_email` = ? AND `user_active` = ? ");
		$stmt->bind_param("ss", $email, $user_active);
		$email = clean_user_input($_POST['email']);
		$user_active = 1;
		$stmt->execute();
		$result = $stmt->get_result();
		$numRows = $result->num_rows;
		if($numRows === 1 ) {
			while($row = $result->fetch_assoc()){
			$db_password = $row['user_password'];
				
			}
			if(password_verify($_POST['password'], $db_password)) {
				echo "you can login";
			
			} else {
				
				echo "incorrect details";
			}
				
		} 

Open in new window

0
NEW Veeam Agent for Microsoft Windows

Backup and recover physical and cloud-based servers and workstations, as well as endpoint devices that belong to remote users. Avoid downtime and data loss quickly and easily for Windows-based physical or public cloud-based workloads!

 
Black SulfurAuthor Commented:
Ah, yes Julian. I saw that while trying to find it but I think maybe you beat me to it as when I came back here to say I had found it your post was here :)

Anyway, is the way I have done it okay or is there a better way of coding it?

Also, I am so paranoid using prepared statements because I believe that I don't have to use real_escape_string
0
 
Black SulfurAuthor Commented:
@ Dave, I don't think you need to do that because you check it in the IF statement that you have to use with password_verify
0
 
Julian HansenCommented:
Hint - always use { } for if statements to avoid this kind of problem. There is no benefit (IMO) to leaving the { } out. In this case your closing } matches the if for the num_rows - so you don't have anything checking the actual password_verify

There are a couple of other things I would change as well see below
// ASSUME NOT AUTH
$auth = false;

// MAKE SURE YOU GOT A PASSWORD
$post_password = empty($_POST['password']) ? false : $_POST['password'];

// SPIN THE CPU's WHEELS ONLY IF YOU HAVE A password
if ($post_password) {
  $stmt = $link->prepare("SELECT `user_email`, `safe_user_id`, `user_password` FROM `db_users` WHERE `user_email` = ? AND `user_active` = ? ");
  $stmt->bind_param("ss", $email, $user_active);
  $email = clean_user_input($_POST['email']);
  $user_active = 1;
  $stmt->execute();
  $result = $stmt->get_result();
  
  // USE THE RESULT TO DETERMINE IF YOU SHOULD PROCEED
  if ($result) {
    // YOU ARE ONLY INTERESTED IN 1 ROW (THERE SHOULD BE AT MOST 1)
    // SO JUST FETCH IT - YOU KNOW THE result IS GOOD SO JUST GET THE row
    $row = $result->fetch_assoc();
  
    // THE CRUX OF THE FUNCTION - CHECK AGAINST YOUR POST PASSWORD
    if (password_verify($post_password, $db_password)) {
    // ALL GOOD SO WE SET THE $auth FLAG TO true
      $auth = true;
    }
  }
}
// NOW WE CHECK TO SEE THE RESULT 
// WE CAN GET HERE BY A NUMBER OF PATHS
// WE ONLY ALLOW ENTRY IF $auth IS TRUE
if ($auth) {
  echo "user authenticated";
}
else {
  echo "Incorrect login details";
}

Open in new window

1
 
Dave BaldwinFixer of ProblemsCommented:
Well, that's the way I check it on many web sites.  If there is no match in the database for the username and the hashed password, then they can't login.
0
 
Black SulfurAuthor Commented:
I don't know how it would work because you have to first get the password field from the database and then you have to use password_verify on the input vs what the actual record found in the database is. In your code example (unless I have just overlooked it), there is no password verify which HAS to be used with password_hash.

But, I am no expert so please correct me if I am wrong because I want to learn the best way. My understanding is that if I created a password using PHP built in function password_hash, I have to use PHP built in function, password_verify.
0
 
Dave BaldwinFixer of ProblemsCommented:
I don't know for sure what the 'best' way is.  All hash functions that I know of create a text output.  The MD5 function in PHP is identical to the MD5 function in MySQL.

When I create a username and password, I store the hash of the password in the database.  When someone submits a username and password to 'login', I hash the password with the same function and see if there is a match to the username and password in the database.  It's certainly simple.  While there is some other code to cleanup the input values, this is the exact code I use to check the database.  It should return one and only one row if there is a match, none if there is not.
//make the password md5
$tmp_pass = md5($tmp_pass);
//check the database
$query = "SELECT * FROM users WHERE username = '$tmp_name' AND password = '$tmp_pass'";
$result = $link->query($query);

Open in new window

0
 
Black SulfurAuthor Commented:
Hi Dave,

yes, agreed. That method works fine with md5, but  not with password_hash and password_verify

http://php.net/manual/en/function.password-hash.php
http://php.net/manual/en/function.password-verify.php
0

Featured Post

Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

  • 5
  • 3
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now