Security::checkHash returns true when using with a non-bcrypt hash

[iby]

$di->setShared('security', function () {
    $security = new Security();
    $security->setWorkFactor(12);
    return $security;
});

…

echo DI::getDefault()->get('security')->checkHash('not jelly beans', 'cb7d86ece76c57eac5ed18420ca67ea0'); // true;

That's so not true… 😰 Though, when using actual incorrect bcrypt hashes it works fine.

[phalcon]

We're not implementing our own bcrypt, we're using php's bcrypt: https://github.com/phalcon/cphalcon/blob/master/ext/security.c#L281

[iby]

I saw that, nevertheless the output is different. I guess something goes wrong in this if security.c#L291?

$crypt1    = crypt('jelly beans', '$2a$12$pB5pV4BxwyF6NEhXZvUuReH3W9q8r6O4vev3zowUbLcR.olErmouy');
// $2a$12$pB5pV4BxwyF6NEhXZvUuReH3W9q8r6O4vev3zowUbLcR.olErmouy
// Hashes match.

$crypt2    = crypt('jelly beans', 'cb7d86ece76c57eac5ed18420ca67ea0');
// cbDA8obfHjP5g
// Here they don't…

[iby]

Am I guessing or do we compare string lengths here:

if (Z_STRLEN_P(hash) == Z_STRLEN_PP(password_hash)) {

But if they are not equal in length first place then this:

RETURN_BOOL(check == 0);

Will always be true. Hence the result.

[phalcon]

Fixed in 1.2.6 and 1.3.0

[chinlung]

In 1.2.6, When checkHash first param is empty, it's always true...
ex:
$this->security->checkHash("", $user->passwd)