password_verify is returning true

Question

I want to compare my password and my hash password with password_verify() but always returns true.Why is that happening?

Here is the code:

if($_SERVER["REQUEST_METHOD"] == "POST") {
    // username and password sent from form 

    $myusername = mysqli_real_escape_string($db,$_POST['username']);
    $mypassword = mysqli_real_escape_string($db,$_POST['password']); 
    $hash = password_hash($mypassword, PASSWORD_DEFAULT);

    $ourdb = "SELECT handle FROM qa_users WHERE handle = '$myusername' and passhash = '$mypassword'";
    $ourresult = mysqli_query($db,$ourdb);
    $ourrow = mysqli_fetch_array($ourresult,MYSQLI_ASSOC);
    $ouractive = $ourrow['active'];
    $ourcount = mysqli_num_rows($ourresult);

    if(password_verify($mypassword, $hash)){
        echo "hashed";
    }

Why should it _not_ return true? Plus: Obligatory "use prepared statements" warning. — Marvin, Jul 10 '19 at 13:42
You are supposed to select the password from the database and compare it to the one entered - you are comparing it against itself(effectively). — Nigel Ren, Jul 10 '19 at 13:45
You shouldn't use `mysqli_real_escape_string` on usernames or passwords, because this can change their value leaving your user unable to actually log in — GrumpyCrouton, Jul 10 '19 at 13:45
@GrumpyCrouton long time no see. Your statement it valid only for the password and only when hashing is used properly. Whereas for the (invalid) code present, the usage is valid. — Your Common Sense, Jul 10 '19 at 13:51
@YourCommonSense True, though generally `mysqli_real_escape_string` is not needed at all, for any value, when using prepared statements. I swear I've seen cases where `mysqli_real_escape_string` changed someones username and they weren't able to log into their site. Also, the hashing in OP's code seems perfectly valid, granted they are verifying incorrectly (of course their use currently is invalid, it doesn't actually verify anything, it effectively has 0 security from just logging in with any password). — GrumpyCrouton, Jul 10 '19 at 14:10
@GrumpyCrouton this could be only the case when mres() usage is completely wrong, like in addition to prepared statements. — Your Common Sense, Jul 10 '19 at 14:12

score 8 · Accepted Answer · edited Jul 10 '19 at 14:01

8

What you're currently doing is hash the password (which you escaped first; you should never escape passwords as that changes the hash), then match/verify it against the value you just hashed, without using the hash from the database - so it will always match. It's the equivalent of setting a variable $a = 'foo';, then checking if ($a == 'foo') - the check will always return true.

Instead, fetch the hashed value from the database based on the username, and use that as the second argument to password_hash().

Also,

Don't compare against the hash in the database, fetch it and then run it through password_verify()
Use prepared statements (instead of the standard query() method and using real_escape_string()) - see How can I prevent SQL injection in PHP?

if($_SERVER["REQUEST_METHOD"] == "POST") {
    $stmt = $db->prepare("SELECT passhash FROM qa_users WHERE handle = ?");
    $stmt->bind_param("s", $_POST['username']);
    $stmt->execute();
    $stmt->bind_result($hash);
    $stmt->fetch();

    if (password_verify($_POST['password'], $hash)) {
        echo "Valid login";
    } else {
        echo "Invalid login";
    }
    $stmt->close();
}

edited Jul 10 '19 at 14:01

Your Common Sense

156,878
40
214
345

answered Jul 10 '19 at 13:45

Qirel

25,449
7
45
62

1

Note to OP: Using the `?` placeholder and `bind_param()` is known as using Prepared Statements, this effectively eliminates the risk of SQL Injection, and it also means you don't have to clean your inputs before using them in the query. – GrumpyCrouton Jul 10 '19 at 13:47
1

*"Don't compare against the hash in the database, fetch it and then run it through password_hash()"* Indeed adding the passhash in the WHERE might cause possible [timing attacks](https://en.wikipedia.org/wiki/Timing_attack) again because databases are designed to return data as quick as possible... Especially when the passhash is included in a index and Btree search algorithm is pretty stable timewise.. – Raymond Nijland Jul 10 '19 at 13:59
1

@RaymondNijland Not only that, the hash is different for each hash - so you will not get matches even if the password is in reality correct. But yes, that too! – Qirel Jul 10 '19 at 14:02
1

indeed i know @Qirel `password_hash()` protects against rainbow tables and using blowfish also protect against GPU (clusters) bruteforcing.. As blowfish is designed to not run fast on CPU's and extremly slow on GPU's if you can get it working. – Raymond Nijland Jul 10 '19 at 14:05

password_verify is returning true

1 Answers1