Just wanna report that the function mt_srand() on line 7748 in the file inc/functions.php gets sometimes a float value instead of a integer, because hexdec() returns on too big values a float instead of a integer. A type casting on line 7748 would fix that problem, since mt_srand() expects an integer (dunno why the doc example even uses float).
mt_srand((int) secure_seed_rng());
Confirmed in MyBB 1.8.6 (I suspect this bug is in all releases).
PHP Warning was thrown in PHP 7.0.1
seems more like you should fix secure_seed_rng() instead... type casting masks too many other issues and might result in bad random numbers.
php documentation even says " Note: There is no need to seed the random number generator with srand() or mt_srand() as this is done automatically. "
so maybe it shouldn't be done at all by now...
(2015-12-28, 04:48 PM)frostschutz Wrote: [ -> ]php documentation even says " Note: There is no need to seed the random number generator with srand() or mt_srand() as this is done automatically. "
so maybe it shouldn't be done at all by now...
That's what I thought after opening the topic too, maybe even not providing a seed might be the better solution than fixing the secure_seed_rng function.
How would you fix the function though? I mean, preventing the warning on 32-bit systems would be possible by wrapping crc32() with abs() to get rid of the overflowing integer before using hexdec(), but not sure how much that affects the randomness and if there are any better ways.
EDIT: nvm, abs() would clearly render the $count parameter useless sometimes on 32-bit systems. But not sure why that parameter exists either, it's not changed in the core anyways. Reducing it to 7 would also fix it, but again, not sure how much that will affect randomness.
(2015-12-28, 10:14 PM)Destroy666 Wrote: [ -> ]How would you fix the function though? I mean, preventing the warning on 32-bit systems would be possible by wrapping crc32() with abs() to get rid of the overflowing integer before using hexdec(), but not sure how much that affects the randomness and if there are any better ways.
EDIT: nvm, abs() would clearly render the $count parameter useless sometimes on 32-bit systems. But not sure why that parameter exists either, it's not changed in the core anyways. Reducing it to 7 would also fix it, but again, not sure how much that will affect randomness.
You don't. You'll let PHP generate the seed itself. There is no need to initialize a seed with mt_srand(), since it is done automatically by PHP. That means the function secure_seed_rng() renders useless, which can be removed if it isn't used anywhere else.
http://php.net/mt_srand
(2015-12-28, 04:48 PM)frostschutz Wrote: [ -> ]php documentation even says " Note: There is no need to seed the random number generator with srand() or mt_srand() as this is done automatically. "
so maybe it shouldn't be done at all by now...
Not since PHP 5.4, but MyBB 1.8's requirements are set to >= 5.2.
(2015-12-28, 10:14 PM)Destroy666 Wrote: [ -> ]How would you fix the function though? I mean, preventing the warning on 32-bit systems would be possible by wrapping crc32() with abs() to get rid of the overflowing integer before using hexdec(), but not sure how much that affects the randomness and if there are any better ways.
That would cut the entropy in half, make 0 less probable to be returned and the integer would still fallback to float on
-2,147,483,648 (regarding
abs(crc32()) only).
Quote:EDIT: nvm, abs() would clearly render the $count parameter useless sometimes on 32-bit systems. But not sure why that parameter exists either, it's not changed in the core anyways. Reducing it to 7 would also fix it, but again, not sure how much that will affect randomness.
The
$count parameter is supposed to control the byte length of the output, but only affects the internal seeding (the function output is a 32-bit, or 4-byte number) - the
hexdec(substr(dechex(crc32(base64_encode($output))), 0, $count)) part doesn't make much sense.
The subsequent functions (
OpenSSL,
mcrypt,
/dev/urandom or
CAPICOM, and the internal one if everything else fails) should simply return binary data of specified length and the function should convert it to a signed integer (so the
$count parameter should be removed and the function should set it to 4 for 32-bit systems and 8 for 64-bit systems and try again if the value is equal to
PHP_INT_MAX + 1, which would still fallback to type float).
Additionally,
my_rand() can seed into
mt_srand() only for PHP < 5.4.
It also should get binary data from cryptographically secure sources like
secure_seed_rng() does on every call instead of relying on internal obfuscators and unautited code since it's being used for authorization.
(2015-12-29, 01:49 AM)Exception Wrote: [ -> ]You don't. You'll let PHP generate the seed itself. There is no need to initialize a seed with mt_srand(), since it is done automatically by PHP. That means the function secure_seed_rng() renders useless, which can be removed if it isn't used anywhere else.
I know it can be done automatically but I asked for an alternative.. Not sure how "strong" PHP's default randomness is:
https://github.com/php/php-src/blob/2509....h#L50-L54 but I guess that a new seed function was introduced for a reason since the parameter could be always skipped.
(2015-12-29, 02:12 AM)Devilshakerz Wrote: [ -> ]Additionally, my_rand() can seed into mt_srand() only for PHP < 5.4.
It also should get binary data from cryptographically secure sources like secure_seed_rng() does on every call instead of relying on internal obfuscators and unautited code since it's being used for authorization.
Well, that would be a good compromise to actually use PHP's potential at default and only fallback to own function to get somewhat something secure when the PHP installation isn't actually supported anymore (any hoster which still hosts websites with unsupported php version should get hit in the face with a chair).
(2015-12-29, 02:23 AM)Destroy666 Wrote: [ -> ] (2015-12-29, 01:49 AM)Exception Wrote: [ -> ]You don't. You'll let PHP generate the seed itself. There is no need to initialize a seed with mt_srand(), since it is done automatically by PHP. That means the function secure_seed_rng() renders useless, which can be removed if it isn't used anywhere else.
I know it can be done automatically but I asked for an alternative.. Not sure how "strong" PHP's default randomness is: https://github.com/php/php-src/blob/2509....h#L50-L54 but I guess that a new seed function was introduced for a reason since the parameter could be always skipped.
Usually it's good, but that also requires a up to date php version, not an unsupported version which is 9 years old (the support ended almost 5 years ago!). But since 5.2.1 there is a new algorithm for the PHP default seeding, which is generally better.
Most hosters upgraded their servers to at least 5.4, which is nice, but still not good enough. Even the support for PHP 5.5 will end in 6 months.