Type checks of custom salt too permissive

[Jacques1]

Why is null or a boolean a valid salt? Sure, it will be caught later by the length check, but that's only because those values happen to have a string representation with less than 22 characters. Semantically, this makes no sense.

Why is a number a valid salt? 22 decimal digits are hardly acceptable.

Is it really a good idea to accept objects? Just because it has a __toString() method doesn't mean it was actually intended to mimic a string. This is very error-prone, because the user might accidentally pass some object which merely contains the salt string (an instance of a randomness class or whatever).

In my opinion, the function should only accept a real, actual string. This might be against “the PHP way”, but I don't think a security-related function is the right place for type juggling, duck typing or whatever.

[Voyager-Two]

Why is the custom salt feature even there?

[Jacques1]

Because different systems have different ways of generating strong random numbers. The automatic salt generation tries out a few possible sources, but it can't cover every single server setup on this planet.

Linux is different from Windows. A new Windows is different from an old one. OpenBSD is different from both Linux and Windows and also has differences accross its versions. Maybe the server only has a hardware RNG. Nobody knows, which is why there's an option to manually generate the salt.

[sagikazarmark]

I think this is something that is caused by PHP's bad behaviour. In PHP you can pass any scalar values (+null) where the documentation says string. There are only a few exceptional cases. In a backported function, the same functionality must be provided. So if PHP does it badly, ...

[Lewiscowles1986]

Not only is the custom salt bad, rather than forcing openssl or mcrypt, falling back to /dev/urandom (just use /dev/urandom, maintain less code?). There is a fourth option available in the code that should at the very least generate a notice; to say it is not an invariant as it uses ^ ord(mt_rand())...
See http://phpsecurity.readthedocs.org/en/latest/Insufficient-Entropy-For-Random-Values.html