Fixed bug #75170

This change may result in different mt_rand/rand sequences being
generated on 64-bit systems for a specific seed.

See also https://externals.io/message/100229.

NEWS

@@ -30,6 +30,9 @@ PHP NEWS
  . Fixed bug #75155 (AppendIterator::append() is broken when appending another
  AppendIterator). (Nikita)
 
+- Standard:
+ . Fixed bug #75170 (mt_rand() bias on 64-bit machines). (Nikita)
+
 - ZIP:
  . Fixed bug #75143 (new method setEncryptionName() seems not to exist
  in ZipArchive). (Anatol)

UPGRADING

@@ -94,6 +94,10 @@ PHP 7.2 UPGRADE NOTES
  sessions. Use output buffer just like web applications to resolve problems on
  CLI scripts.
 
+- Standard:
+ . Sequences generated by mt_rand() and rand() for a specific seed may differ
+ from PHP 7.1 on 64-bit machines. This change was necessary to resolve a
+ modulo bias bug in the implementation.
 
 ========================================
 2. New Features

ext/standard/mt_rand.c

@@ -212,50 +212,81 @@ PHP_FUNCTION(mt_srand)
 }
 /* }}} */
 
-/* {{{ php_mt_rand_range
- */
-PHPAPI zend_long php_mt_rand_range(zend_long min, zend_long max)
-{
- zend_ulong umax = max - min;
- zend_ulong limit;
- zend_ulong result;
+static uint32_t rand_range32(uint32_t umax) {
+ uint32_t result, limit;
 
  result = php_mt_rand();
-#if ZEND_ULONG_MAX > UINT32_MAX
- if (umax > UINT32_MAX) {
- result = (result << 32) | php_mt_rand();
- }
-#endif
 
  /* Special case where no modulus is required */
- if (UNEXPECTED(umax == ZEND_ULONG_MAX)) {
- return (zend_long)result;
+ if (UNEXPECTED(umax == UINT32_MAX)) {
+ return result;
  }
 
  /* Increment the max so the range is inclusive of max */
  umax++;
 
  /* Powers of two are not biased */
- if (EXPECTED((umax & (umax - 1)) != 0)) {
- /* Ceiling under which ZEND_LONG_MAX % max == 0 */
- limit = ZEND_ULONG_MAX - (ZEND_ULONG_MAX % umax) - 1;
+ if ((umax & (umax - 1)) == 0) {
+ return result & (umax - 1);
+ }
+
+ /* Ceiling under which UINT32_MAX % max == 0 */
+ limit = UINT32_MAX - (UINT32_MAX % umax) - 1;
+
+ /* Discard numbers over the limit to avoid modulo bias */
+ while (UNEXPECTED(result > limit)) {
+ result = php_mt_rand();
+ }
+
+ return result % umax;
+}
 
- /* Discard numbers over the limit to avoid modulo bias */
- while (UNEXPECTED(result > limit)) {
 #if ZEND_ULONG_MAX > UINT32_MAX
- if (umax > UINT32_MAX) {
- result = (result << 32) | php_mt_rand();
- }
- else {
- result = php_mt_rand();
- }
-#else
- result = php_mt_rand();
+static uint64_t rand_range64(uint64_t umax) {
+ uint64_t result, limit;
+
+ result = php_mt_rand();
+ result = (result << 32) | php_mt_rand();
+
+ /* Special case where no modulus is required */
+ if (UNEXPECTED(umax == UINT64_MAX)) {
+ return result;
+ }
+
+ /* Increment the max so the range is inclusive of max */
+ umax++;
+
+ /* Powers of two are not biased */
+ if ((umax & (umax - 1)) == 0) {
+ return result & (umax - 1);
+ }
+
+ /* Ceiling under which UINT64_MAX % max == 0 */
+ limit = UINT64_MAX - (UINT64_MAX % umax) - 1;
+
+ /* Discard numbers over the limit to avoid modulo bias */
+ while (UNEXPECTED(result > limit)) {
+ result = php_mt_rand();
+ result = (result << 32) | php_mt_rand();
+ }
+
+ return result % umax;
+}
 #endif
- }
+
+/* {{{ php_mt_rand_range
+ */
+PHPAPI zend_long php_mt_rand_range(zend_long min, zend_long max)
+{
+ zend_ulong umax = max - min;
+
+#if ZEND_ULONG_MAX > UINT32_MAX
+ if (umax > UINT32_MAX) {
+ return (zend_long) (rand_range64(umax) + min);
  }
+#endif
 
- return (zend_long)((result % umax) + min);
+ return (zend_long) (rand_range32(umax) + min);
 }
 /* }}} */
 

ext/standard/tests/math/bug75170.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Bug #75170: mt_rand() bias on 64-bit machines
+--CREDITS--
+Solar Designer in https://externals.io/message/100229
+--FILE--
+<?php
+
+// PHP pre-7.1.0 modulo bias
+mt_srand(1234567890);
+$total = 10000;
+$max = 0x66666666;
+$halves[0] = $halves[1] = 0;
+for ($i = 0; $i < $total; $i++) {
+ $halves[(mt_rand(0, $max - 1) >> 1) & 1]++;
+}
+printf("%.1f%% vs. %.1f%%\n", 100. * $halves[0] / $total, 100. * $halves[1] / $total);
+
+// PHP 7.1.0 to 7.2.0beta2 modulo bias bug found during work
+// on http:https://www.openwall.com/php_mt_seed/
+mt_srand(1234567890);
+$total = 10000;
+$max = 0x66666666;
+$halves[0] = $halves[1] = 0;
+for ($i = 0; $i < $total; $i++) {
+ $halves[mt_rand(0, $max - 1) / ($max / 2)]++;
+}
+printf("%.1f%% vs. %.1f%%\n", 100. * $halves[0] / $total, 100. * $halves[1] / $total);
+
+?>
+--EXPECT--
+49.5% vs. 50.5%
+50.5% vs. 49.5%