Update phpass to version 0.2 (check /dev/urandom before accessing it)

[hakre]

the phpass class is using the @ error operator to suppress messages when /dev/urandom is not accessible but does not check wether it is readable before.

accessing such a resource on systems where it does not exists (win32/winnt operating systems) this will lead to a warning.

this warning is suppresd by the @ operator but handeled over to the error handler anyway. it therefore stands in the way if you implement own error handlers and throw exceptions then like

set_error_handler(create_function('$errno, $errstr, $errfile, $errline', 'throw new ErrorException($errstr, 0, $errno, $errfile, $errline);'));

the @ operator is considered bad practice not only because of that and it's usage should be reduced.

attached you will find a patch that is preventing errors on windows systems (and others where /dev/urandom is not accessible) and therefore should improve it.

I contacted the class author as well so there is a chance to have this upstreamed.

[hakre]

I had contact with the class author and he decided positivly for the patch per se. I think this should go clean here & upstream in parallel and therefore test & green while this is patched directly here. It has been even checked being ​PHP 3 compatible. Just as a sidenote.

[hakre]

phpass has been updated to version 0.2 containing that change. I had not noticed that in the time of my last comment. so this can be updated in wordpress as well. I will create an updated patch that reflects these changes.

[hakre]

upgrade of phpass to version 0.2

[hakre]

second patch is upgrading to the actual version. one part of the file has been untouched:

$this->random_state = microtime() . (function_exists('getmypid') ? getmypid() : '') . uniqid(rand(), TRUE);

The original code does not check for getmypid here. I assume this check is made because some hosters might disable that function. I suggest to put that into pluggable or into some other layer sothat the class can be kept untouched and the fix is provided globally.

[hakre]

forgot to mention. original code is:

$this->random_state = microtime() . getmypid();

[dd32]

$this->random_state = microtime() . (function_exists('getmypid') ? getmypid() : ) . uniqid(rand(), TRUE);

IMO, the usage of getmypid() should be removed entirely, Its not a reliable source of crypto-safe random data.. This is due to Process ID's having weak entropy..

To quote the PHP Manual even: "Process IDs are not unique, thus they are a weak entropy source. We recommend against relying on pids in security-dependent contexts."

I'd support removal of the getmypid() branch all together, and rely upon uniqid(rand(), TRUE) instead, as its going to be actually random.

[hakre]

Yeah I read that as well in the manual. I will update the patch and propose the changes upstream.

[hakre]

Updated for compability reasons

[westi]

Don't want to change this late in the dev cycle.

[ryan]

[12521]