Crypt function doesn't work when comparing string to hash

Question

I'm using a pretty standard way of cookie login - I give the user two cookies, one with his username and the other with a randomly generated string plus a user-specific salt.

This is what happens at login:

$_SESSION['username']=$row[username];
$_SESSION['user_id']=$row['id'];
$loginhash=generateRandomBase64String()."_".$row['salt'];
$number_of_days = 14;
$date_of_expiry = time() + 60 * 60 * 24 * $number_of_days ;
setcookie( "userlogin", $row['username'], $date_of_expiry, "/" ) ;
setcookie( "loginhash", $loginhash, $date_of_expiry, "/" ) ;
$cryptedhash=crypt($loginhash);

$today=date("Y-m-d");

mysql_query("update members set last_login='$today',loginhash='$cryptedhash' where id='$row[id]' ") or die(mysql_error());

So the $loginhash value is something like Pe0vFou8qe++CqhcJgFtRmoAldpuIs+d_g5oijF76 and the crypted version of that is stored in the database. The salt is already in the database, as it's generated for each user when they sign up.

I use session variables ($_SESSION[username]) to keep users logged in. Then, when a user visits the site, I check for two things: if $_SESSION[username] is not set but $_COOKIE[userlogin] is, I check if the hash is correct so I could log the user in. The problem is, the hash is never correct.

if($_COOKIE['userlogin'] && !isset($_SESSION[user_id])){
    $username=mysql_real_escape_string($_COOKIE['userlogin']);
    $loginhash=mysql_real_escape_string($_COOKIE['loginhash']);
    $salt=substr($loginhash,-8);
    $result=mysql_query("select * from members where (username='$username' || email='$username') && salt='$salt' limit 1 ") or die (mysql_error());
    $row=mysql_fetch_assoc($result);
    $cryptedhash=$row['loginhash'];
    if (crypt($loginhash, $cryptedhash) == $cryptedhash){
        $_SESSION['username']=$row[username];
        $_SESSION['user_id']=$row['id'];
    }
}

$_COOKIE[userlogin] is the correct value. When I check for the username/salt combination in the database, the correct result is found (echo $row[username] gives correct value). However, the if condition below that is never met. I would think there's something weird about my PHP configuration, but I use the same crypting mechanism to store passwords and there it works properly.

So can anyone see what's going wrong here?

PS I'm not looking to start a discussion about cookie safety or the variety of available hashing functions here.

Triple check all the involved values, especially what is returned by `crypt($loginhash, $cryptedhash)` and `$cryptedhash`. Is your database field too short to hold the entire hash possibly...? — deceze, Aug 21 '13 at 10:15
The DB field is varchar(64) so definitely long enough. If I take the string from the cookie manually and do `crypt("stringhere", $cryptedhash)`, it still doesn't work. `crypt($loginhash, $cryptedhash)` returns what looks like a properly formatted hash, in this case `$1$8CwqG9Pq$Y.3YmFOhXc78ucBki4GPA.`. — sveti petar, Aug 21 '13 at 10:23
I could be wrong, but wouldn't this line `if (crypt($loginhash, $cryptedhash) == $cryptedhash)` be saysing, if(crypt(hash, salt) == salt)... wouldn't salting the hash make the result not equal to the salt? — ContextSwitch, Aug 27 '13 at 13:38
@ContextSwitch http://www.php.net/manual/en/function.crypt.php The php docs say that you can use that method to verify the hashes — Schleis, Aug 27 '13 at 13:40
Is that you are using an escaped string modifying your input for the hash so that `crypt` thinks the string is different? Try using `crypt` on the unescaped string. Also insert here warning about the `mysql_` php functions. — Schleis, Aug 27 '13 at 13:54
In the code above you have: `!isset($_SESSION[user_id])` and that should be `!isset($_SESSION['user_id'])`. — Benny Hill, Aug 27 '13 at 13:56
can you please show us `print_r($row);` just after `$row=mysql_fetch_assoc($result);` i think you are using salt incorrectly — Saic Siquot, Aug 27 '13 at 14:21
blowfish makes all this easier (fyi). you just call the prebuilt function to salt on registration and then check function when logging them in — James, Sep 02 '13 at 01:26

score 3 · Answer 1 · answered Aug 27 '13 at 15:20

Here's the problem:

In your first call to crypt(), you do not specify a salt. In your second call to crypt(), you pass the $cryptedhash as the salt.

crypt() is documented to generate a random salt if you do not provide one, and then prepend that salt to the returned hash. That has the side effect that if you pass a returned salt+hash as the hash for a subsequent call, crypt() will still pull the correct salt out of it.

Unfortunately the algorithm used and the length/format of the salt+hash is based on a combination of your operating system, PHP version, and whether or not you specified the salt parameter. When you used your code previously, you had the happy accident that DES was chosen in both calls to crypt(). Now, your environment is using a different algorithm for the 2 calls to crypt() since you only supplied the hash in one of them.

The solution is to just pass a consistent salt to both calls to crypt(). You can stop appending the salt to the string you want to hash, and actually pass your user salt as the salt parameter and everything will be fine.

Saic Siquot · Answer 2 · 2013-08-27T14:32:41.807

0

This line is wrong $loginhash=mysql_real_escape_string($_COOKIE['loginhash']); You don't need to escape it (you are not using it with the database), you must use it unaltered when pass it to crypt(), so the line can be writen as $loginhash=$_COOKIE['loginhash']; (at least for this part of code)

edited Aug 27 '13 at 14:32

answered Aug 27 '13 at 13:37

Saic Siquot

6,513
5
34
56

The salt is fixed already, it's stored in the database, unique for each user and unchangeable. – sveti petar Aug 27 '13 at 14:25
Oh, I see you've changed your answers since my comment. But it doesn't fix the issue - it remains the same if I don't escape the string, and it remains the same if instead of `$loginhash` I use the string in the crypt function directly (copy/paste it manually). – sveti petar Aug 27 '13 at 15:17
can you please show `print_r($row);` just after `$row=mysql_fetch_assoc($result);` – Saic Siquot Aug 27 '13 at 15:25

score 0 · Answer 3 · answered Sep 02 '13 at 13:43

If you are using PHP Version 5.5.0 or higher you should take a look at PHPDoc - Password Hashing!
I know you said you don't want to discuss different hashing functions, but I thought this one makes it easyer for you since it should get rid of your problem and is (in my oppinion) way easier to use!

Here would be your code with the new functions: (not tested, if broke pleases comment)

$_SESSION['username']=$row[username];
$_SESSION['user_id']=$row['id'];
$loginhash=generateRandomBase64String()."_".$row['salt'];
$number_of_days = 14;
$date_of_expiry = time() + 60 * 60 * 24 * $number_of_days ;
setcookie( "userlogin", $row['username'], $date_of_expiry, "/" ) ;
setcookie( "loginhash", $loginhash, $date_of_expiry, "/" ) ;
$cryptedhash=password_hash($loginhash, PASSWORD_DEFAULT, array("cost" => 10));
// a cost of 10 is standard, you may want to adjust it according to your hardware (lower/higher cost means faster/slower)

$today=date("Y-m-d");

mysql_query("update members set last_login='$today',loginhash='$cryptedhash' where id='$row[id]' ") or die(mysql_error());

Using PASSWORD_DEFAULT there will make shure that even in future versions the strongest algorithm will always be used.

and

if($_COOKIE['userlogin'] && !isset($_SESSION[user_id])){
    $username=mysql_real_escape_string($_COOKIE['userlogin']);
    $loginhash=$_COOKIE['loginhash']; // i guess you should not use mysql_real_escape_string 
    $salt=mysql_real_escape_string(substr($loginhash,-8)); // here would be the place to use it
    $result=mysql_query("select * from members where (username='$username' || email='$username') && salt='$salt' limit 1 ") or die (mysql_error());
    $row=mysql_fetch_assoc($result);
    $cryptedhash=$row['loginhash'];
    if (password_verify($loginhash, $cryptedhash)){
        $_SESSION['username']=$row[username];
        $_SESSION['user_id']=$row['id'];
    }
}

Crypt function doesn't work when comparing string to hash

3 Answers3