Closed Shea690901 closed 9 years ago
Those calls may be strictly unnecessary, however there was a reason for it. At one point I checked the $strong
parameter of openssl_random_pseudo_bytes. Which I really should again... I'll make that change (which then will still require the extra strlen calls).
Thanks for bringing it up!
I'll look forward to those changes :)
Pushed!
That was fast ;)
When 'password_hash' generates the salt itself it might encounter a small glitch (no real problem, only unnecessary calls) :
Assuming it calls either (or both) of »mcrypt_create_iv« or »openssl_random_pseudo_bytes« »$buffer_valid« could still be FALSE after both if's meaning it might try urandom next.
Sadly buffer now is no longer an empty string but »FALSE« itself! While it will give the correct result, it is an unnecessary call to »_strlen« just to get 0. Instead »$read« could be initialized to 0 directly.
Also you are using a while-loop to read from urandom, which won't terminate until we have read enough, meaning the last if for this block is unnecessary, we already know that »$read« is greater or equal to »$raw_salt_len« and we can simply set »$buffer_valid« as true…
Next: past this point we have either a valid buffer, which per definition has a length of at least »$raw_salt_len«, or it's not valid, making the length check for the following if unnecessary… But, like the case of trying urandom, »$buffer« should be reinitialized as '' for clean code and the loop could be simplified, alas we know that we are beyond the end of »$buffer«…
For »password_verify« it's also possible to eliminate 2 calls (one of them inside the loop) to »_strlen« by caching the length of »$ret«.
Hope I haven't overlooked anything and would like to hear your opinion on my changes…