17

I'm using crypt() to hash passwords in PHP, and am trying to work out the safest way of testing equality of the resulting hash when performing password checks.

There are three options that I can see:

Option 1 - Double Equals

function checkPassword($hash, $password)
{
    return crypt($password, $hash) == $hash;
}

Option 2 - Triple Equals

function checkPassword($hash, $password)
{
    return crypt($password, $hash) === $hash;
}

Option 3 - strcmp()

function checkPassword($hash, $password)
{
    return strcmp(crypt($password, $hash), $hash) === 0;
}

My intuition tells me that option 1 is a bad idea, due to the lack of type checking, and that options 2 or 3 are likely to be better. However, I can't work out if there's a specific case that === or strcmp would fail under. Which is safest for this purpose?

Polynomial
  • 27,674
  • 12
  • 80
  • 107
  • 1
    Read it: http://raz0r.name/vulnerabilities/simple-machines-forum/ – user956584 Feb 05 '13 at 16:05
  • @Userpassword Interesting bug! I wonder how PHP handles strings like `$2a$10$...$` when cast as numbers... – Polynomial Feb 05 '13 at 16:08
  • 1
    If you just want to compare hashes reliably, simply use `===`. If you really care about security and potential timing attacks (even despite of network jitter), you should take a look at [this](https://github.com/delight-im/Faceless/issues/4) or [this](https://github.com/delight-im/Faceless/pull/5) discussion or use the `hash_equals()` function (PHP 5.6+). – caw Jul 17 '14 at 00:57
  • check http://danuxx.blogspot.com/2013/03/unauthorized-access-bypassing-php-strcmp.html – Rptk99 Aug 19 '16 at 10:25

4 Answers4

21

When it comes to security I prefer to use the === operator. === ensures the two operands are exactly the same, without trying to accomodate some casting in order to "help" the comparison to reach a successful match - as it may help while developing thanks to a loose-typed language, like PHP.

Of course, one of the operand is to be trusted. A hash from the database is trustable, while the user input is not.

One can always dither for a while, coming to the conclusion there is no risk using == in a specific case. Maybe. But for instance

  "0afd9f7b678fdefca" == 0 is true
  "aafd9f7b678fdefca" == 0 is also true

as PHP tries to convert the "hash" into a number (probably using atoi) which gives 0. While it is unlikely crypt returns 0, I'd prefer to maximize the cases where the passwords don't match (and answer a support call) by using ===, than allowing a rare case that I didn't think about by using ==.

As for strcmp, the function returns <0 or >0 if different, and 0 if equal. But

  strcmp("3", 0003) returns 0
  strcmp("0003", 0003) returns -3

which are not surprising after all. A literal 0003 is actually an integer, 3 and since strcmp expects a string, the 3 will be converted to "3". But that shows there is some conversion that may happen in this case, since strcmp is a function, while === is part of the language.

So my preference in that case goes to === (which is faster than == anyway).

Déjà vu
  • 28,223
  • 6
  • 72
  • 100
  • This is the most sensible answer I've seen. I agree that both type and value are important to avoid issues with the "cast helper". – Polynomial Feb 05 '13 at 17:00
  • Note that [hash_equals() should be used](http://stackoverflow.com/a/27066499/978756) when testing hash values produced with a strong function like `password_hash`. – Polynomial Nov 27 '14 at 16:14
7

You should be using the hash_equals() function that is built into PHP. There would be no need to make your own function. The hash_equals() will return a boolean value.

In my opinion it is usually NOT a good idea to use == or === for comparing strings let alone hashed strings.

bradc14
  • 113
  • 2
  • 10
  • 1
    This is now the correct answer for values generated by `password_hash`, but `hash_equals` didn't exist in mainstream PHP at the time of asking, and it doesn't apply to flat hashes (which shouldn't be used anyway but are sometimes required for legacy) – Polynomial Nov 27 '14 at 16:12
0

That is incorrect, please look at the definition of the function. According to PHP:

Returns < 0 if str1 is less than str2;

> 0 if str1 is greater than str2,

and 0 if they are equal

It returns less than 0 if str1 is less than str2. Note the phrase "less than", it does not return just -1, but any negative value. The same happens when str1 is greater than str2, but it returns a positive, non-zero value. It returns a positive value that can be 1, or any number thereafter.

strcmp()returns a number that is the difference between the two strings starting with the last character that was found to be similar.

Here is an example:

$output = strcmp("red", "blue");

The variable $output with contain a value of 16

Nadeem
  • 419
  • 4
  • 13
-5

I think that using == would be sufficient in your case.

== checks for equality regardless of type, whereas === checks for equality as well as type.

1 == "1" = True

1 === "1" = False

Since we're not too concerned with type, I'd keep it simple and go with ==.

Ian P
  • 12,840
  • 6
  • 48
  • 70
  • 1
    I'm aware of the differences between `==` and `===`, but I'm trying to work out if there's a specific security-sensitive case that I'm missing. For example, is there a reason *not* to use `strcmp`? – Polynomial Feb 05 '13 at 15:59
  • Since you're not concerned with a binary safe comparison, I wouldn't use strcmp. You're not concerned with how different the strings are -- you're just concerned with whether or not they are semantically the same. – Ian P Feb 05 '13 at 16:01
  • What if `bcrypt` fails for some reason? Might it be possible for a non-string return value to be considered equal to a string value for `$hash`, where that variable is a valid hash? – Polynomial Feb 05 '13 at 16:02
  • In short, no. It should not make any different. – Ian P Feb 05 '13 at 16:04
  • $hash = function_returing_0(); var_dump(('00somehash' == $test)); returns true so using only double == without a string cast is a security hole.. – gries Feb 05 '13 at 16:08
  • Well, then the question becomes "Can $hash be zero?", or .. Where does $hash come from? – Ian P Feb 05 '13 at 16:09
  • @gries In this case the hashes will always start with `$2a$10` so I *think* we can discount that issue, but it pays to be safe. – Polynomial Feb 05 '13 at 16:09
  • yeay as long as the hash algorithm does not change, and I'm sure if this is the case no one will notice the problem with comparing == without a cast – gries Feb 05 '13 at 16:10
  • if( "asdf" == 0 ) die("d'oh\n"); – Antti Rytsölä Feb 05 '13 at 16:11
  • 1
    @IanP no thats not the question, he needs a function that securly compares 2 strings, we should NOT care about what the strings "should" or "may" look like, because these things can change – gries Feb 05 '13 at 16:12