crypt broken when salt is 'too' long

Bug #73058	crypt broken when salt is 'too' long
Submitted:	2016-09-09 13:35 UTC	Modified:	2016-09-10 01:18 UTC
From:	sjon at hortensius dot net	Assigned:	ab (profile)
Status:	Closed	Package:	hash related
PHP Version:	7.1.0RC1	OS:
Private report:	No	CVE-ID:	None

[2016-09-09 13:35 UTC] sjon at hortensius dot net

Description:
------------
$pass = 'secret';
$salt = '$2y$07$usesomesillystringforsalt$';

var_dump(crypt($pass, $salt));

* as demonstrated on https://3v4l.org/kuAJO

Test script:
---------------
* works with shorter salt: https://3v4l.org/O654F
* fails with longer salt: https://3v4l.org/dvgnq (includes CRYPT_SALT_LENGTH)

Expected result:
----------------
string(60) "$2y$07$usesomesillystringforex.u2VJUMLRWaJNuw0Hu2FvCEimdeYVO"

Actual result:
--------------
string(2) "*0"

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2016-09-09 14:23 UTC] requinix@php.net

-Status: Open +Status: Verified -Assigned To: +Assigned To: ab

[2016-09-09 15:22 UTC] cmb@php.net

Indeed, Damian.

Anatol, with regard to the fix[1]: it seems to me, that it would
suffice to check that the actual salt is not empty, i.e. to do the
following instead:

    if (salt[7] == '$') {
        return NULL;
    }

[1] <https://github.com/php/php-src/commit/295303b5>

[2016-09-10 01:18 UTC] ab@php.net

-Status: Verified +Status: Closed

[2016-09-10 01:18 UTC] ab@php.net

Christoph,

yeah, that's a better approach. I also completely forgot about the hacks with '$' in crypt(). Re-fixed with 669fda00b75a0d361810429e0ef53f6c740b1727.

Thanks.