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.

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.

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.

upgrade of phpass to version 0.2

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.

forgot to mention. original code is:

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

$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.

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

Updated for compability reasons

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

[12521]