2

I've cribbed this code almost verbatim from a bunch of very helpful answers here on SO, so I can't get my head around what's wrong.

First, here's my function for creating a user account:

function BFcrypt($password,$cost)
{
    $chars='./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
    $salt=sprintf('$2a$%02d$',$cost);
    for($i=0;$i<22;$i++) $salt.=$chars[rand(0,63)];
    return array(
            'salt'=>$salt,
            'hash'=>crypt($password,$salt)
            );
}

Then, when a user goes to login:

case 'login':
    $login  =$_POST['login'];
    $pwd    =$_POST['pwd'];
    $sql    ="SELECT * FROM `users` WHERE `users`.`login`='$login' LIMIT 1;";
    if($query = mysql_query($sql)){
            $row=mysql_fetch_assoc($query);
            print_r($_POST);
            print_r($row);
            $hash = $row['password'];
            if(crypt($pwd,$hash)==$hash){
                echo"SUCCESS";
            }else{
                echo"FAILURE";

            }
    }

The login function appears to always be failing. I've set it to show me $pwd, $hash and crypt($pwd,$hash), and for some reason, crypt($pwd,$hash) never seems to == $hash.

Here's a row in the database for a sample user (I'm logging the salt now, though I know it's supposed to be included in the hash:

'id'=>'680',
'login'=>'argh',
'password'=>'$2a$10$BWZAX7wrwQp5iyK4kh6VLunqy82eiXg7GaDs6mJLqdgT5s2qiUqYW',
'salt'=>'$2a$10$BWZAX7wrwQp5iyK4kh6VL5',
'first'=>'argh',
'last'=>'argh',
'zip'=>'00000',
'email'=>'argh',
'date updated'=>'2012-12-12 16:05:29'

I believe that when I call crypt($pwd,$hash),it truncates $hash, leaving only the original 22-character salt (plus prefix), thus the output will be the same as $hash as long as $pwd is the same. I'm seeing clearly there's an issue here in that the salt I'm recording is one character longer than the one that ends up appended to the hash, but it's the appropriate length for blowfish, and anyway, making it one character shorter doesn't seem to help.

I can't figure out what I'm doing wrong here. Any help would be appreciated.

Charles
  • 50,943
  • 13
  • 104
  • 142
Polisurgist
  • 340
  • 2
  • 11
  • why use ciphering instead of hashing for passwords? and why the salt if not hashing? rainbow attacks are only applicable for hashes, i thought? – Janus Troelsen Dec 12 '12 at 22:32
  • 1
    Crypt does generate a hash. – Polisurgist Dec 12 '12 at 22:37
  • @Polisurgist, please note that code blocks require an empty line before and after in order to render correctly. Also, you should be using `2y` now, not `2a`. You may wish to [review the `crypt` manual page](http://php.net/crypt). – Charles Dec 13 '12 at 08:33
  • 1
    **WARNING!** Your code contains an [SQL injection vulnerability](http://en.wikipedia.org/wiki/SQL_injection) -- you're passing unfiltered user input ($_POST) directly into an SQL string. Please [switch to PDO](http://php.net/book.pdo) or [mysqli](http://php.net/book.mysqli) so you can use [prepared statements with parameterized queries](http://en.wikipedia.org/wiki/Prepared_statement). – Charles Dec 13 '12 at 08:33
  • Can you make it work without going through the database? `$hash = crypt($passwd, $salt); echo $hash == crypt($passwd, $hash);`? – deceze Dec 13 '12 at 08:37
  • The function definitely does work when I skip the database, so it has to do with that. I'm storing the whole hash, though, so I don't know what the deal is still. I'm going to revise how I'm storing this to deal with the aforementioned injection vulnerability and see if I can fix that at the same time. – Polisurgist Dec 13 '12 at 17:04
  • Could you show the output of `var_dump($_POST)` and `var_dump($row)` please? I don't know what you dumped there, but it's not helpful enough :) – Ja͢ck Dec 13 '12 at 23:12

1 Answers1

2

Based on your own salt value and password of 'argh' I ran a small test script:

$hash = crypt('argh', '$2a$10$BWZAX7wrwQp5iyK4kh6VL5');
// $2a$10$BWZAX7wrwQp5iyK4kh6VLuIzJHihvZTdfpRXNkTPVKkTiGfLDl1RO

var_dump(crypt('argh', $hash) == $hash);
// bool(true)

The problem doesn't seem to be in the code you've shown.

You could check your database field width to store the password hash, which should be at least 60 wide. And while you're at it, fix your SQL injection vulnerability (by using prepared statements most preferably).

Ja͢ck
  • 170,779
  • 38
  • 263
  • 309
  • It's definitely storing the whole hash, so I know that isn't the problem, but I know I'm also only having this issue when I'm comparing it to the stored hash. Running the two comparisons side by side shows one works and the other doesn't. – Polisurgist Dec 13 '12 at 17:05
  • Do a var_dump() instead of print_r() btw. – Ja͢ck Dec 13 '12 at 19:38
  • The problem does seemed to be with my writing the hashed password to the database and not with this code at all. With that changed, the script is working correctly now. – Polisurgist Dec 14 '12 at 16:55