?
avatar photodude
photodude
16 Oct 2016

Steps to reproduce the issue

Primarily an issue with php 7.0.0 - 7.1.0RC3 and hhvm, where the result of JUserHelper::getCryptedPassword() could be *0 and/or Deprecated: crypt(): Supplied salt is not valid for DES indicating a failure when the salt is malformed or too short

Use the following

$encryption = 'crypt-blowfish';
$plaintext = 'mySuperSecretPassword';
$salt = '';
$newSalt = JUserHelper::getSalt($encryption, $salt, $plaintext);

Expected result

The salt for crypt-blowfish should be correctly formed as per the php manual crypt() instructions

The expected size is at least 30 characters in the form

prefix cost parameter - a base-2 logarithm of the hashing iteration count string
$2y$ $[04-31] $ . 22 characters from the alphabet [./0-9A-Za-z]

For example, a complete salt should look like '$2a$07$usesomesillystringforsalt$'

Actual result

The salt generated for crypt-blowfish is only 16 characters in length. and is missing necessary information.

mock examples with failures of a mocked version of JUserHelper::getCryptedPassword()
https://3v4l.org/Is24j

System information

Primarily an issue with php 7.0.0 - 7.1.0RC3 and hhvm, php less than 5.6 seems to not have an issue with 16 char salts

Additional comments

salt generated at these lines https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/helper.php#L569-L578

We also need to adjust the unit test JUserHelperTest::testGetCryptedPassword to expect 60 characters rather than 13 characters

There maybe BC issues for anything previously stored with the old method

avatar photodude photodude - open - 16 Oct 2016
avatar photodude photodude - change - 16 Oct 2016
The description was changed
avatar photodude photodude - edited - 16 Oct 2016
avatar photodude photodude - change - 16 Oct 2016
The description was changed
avatar photodude photodude - edited - 16 Oct 2016
avatar photodude photodude - change - 16 Oct 2016
The description was changed
avatar photodude photodude - edited - 16 Oct 2016
avatar brianteeman brianteeman - close - 17 Oct 2016
avatar photodude photodude - change - 17 Oct 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-10-17 16:29:10
Closed_By photodude
avatar photodude photodude - close - 17 Oct 2016
avatar photodude
photodude - comment - 17 Oct 2016

closing the issue since I opened a PR

avatar brianteeman brianteeman - change - 19 Oct 2016
Labels Added: ?

Add a Comment

Login with GitHub to post a comment