Re: PHP 7.1.0 to 7.2.0beta2 mt_rand() modulo bias bug

On Wed, 16 Aug 2017 at 20:13 Solar Designer <solar@openwall.com> wrote: > Also, why even bother to support ranges beyond 32-bit? Sounds like a > misfeature to me, considering it won't(?) be universally available on > all PHP builds anyway (not on 32-bit ones, right?) and thus shouldn't(?) > be relied upon by applications (although it might become reasonable for > application developers not to care about 32-bit soon). I also see few > use cases for it, even if it were universally available. > It was possible (on 64 bit builds) to specify min and max such that the size of the output required from mt_rand was the full 64 bit range. Prior to 7.1 this full output was created by stretching a single 32 bit output up to the required range using floating point arithmetic, which caused other biases in the output. Unfortunately when fixing this bias, a new bias was introduced. I took known working code from the CSPRNG and didn't account for the variable length of the sample. My proposed fix would be to add a "limit_max" variable, initialise it to UINT32_MAX, and in the first range check where we decide to add an extra output or not, set it to ZEND_ULONG_MAX. Then the statement creating the ceiling value can use limit_max instead of the constant value.