Skip to content

Commit ab834f4

Browse files
ltlt
committed
Merge RNG fixes RFC. PR #1986
* rng-fixes: Fix legacy mode RAND_RANGE and 32/64-bit consistency Fix crypt salt not being converted to b64 Make mode selection part of mt_srand() Use zend_bitset Improve array_rand distribution Fix some insecure usages of php_rand Alias rand to mt_rand Fix RAND_RANGE for mt_rand Fix mt_rand impl. Provide legacy impl. access. Split rand and mt_rand into separate files
2 parents 69f4682 + 027375d commit ab834f4

File tree

18 files changed

+524
-422
lines changed

18 files changed

+524
-422
lines changed

‎NEWS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ PHP NEWS
2323
phpize on Windows). (Yuji Uchiyama)
2424
. Fixed bug #29368 (The destructor is called when an exception is thrown from
2525
the constructor). (Dmitry)
26+
. Implemented RFC: RNG Fixes. (Leigh)
2627

2728
- COM:
2829
. Fixed bug #72569 (DOTNET/COM array parameters broke in PHP7). (Anatol)

‎UPGRADING‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ PHP 7.1 UPGRADE NOTES
4949
the notice level only.
5050
. Don't call destructors of incompletely constructed objects, even if they
5151
are kept referenced. See bug #29368 and Zend/tests/bug29368_1.phpt.
52+
. rand() and srand() are now aliases of mt_rand() and mt_srand().
53+
Consequently the output of the following functions has changed:
54+
. rand()
55+
. shuffle()
56+
. str_shuffle()
57+
. array_rand()
58+
. Fixes to random number generators mean that mt_rand() now produces a
59+
different sequence of outputs to previous versions. If you relied on
60+
mt_srand() to produce a deterministic sequence, it can be called using
61+
mt_srand($seed, MT_RAND_PHP) to produce old the sequences.
5262

5363
- JSON:
5464
. The serialize_precision is used instead of precision when encoding double

‎ext/soap/php_http.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
#include "php_soap.h"
2323
#include "ext/standard/base64.h"
2424
#include "ext/standard/md5.h"
25-
#include "ext/standard/php_rand.h"
25+
#include "ext/standard/php_random.h"
2626

2727
static char *get_http_header_value(char *headers, char *type);
2828
static zend_string *get_http_body(php_stream *socketd, int close, char *headers);
@@ -639,11 +639,15 @@ int make_http_soap_request(zval *this_ptr,
639639
if ((digest = zend_hash_str_find(Z_OBJPROP_P(this_ptr), "_digest", sizeof("_digest")-1)) != NULL) {
640640
if (Z_TYPE_P(digest) == IS_ARRAY) {
641641
char HA1[33], HA2[33], response[33], cnonce[33], nc[9];
642+
zend_long nonce;
642643
PHP_MD5_CTX md5ctx;
643644
unsigned char hash[16];
644645

646+
php_random_bytes_throw(&nonce, sizeof(nonce));
647+
nonce &= 0x7fffffff;
648+
645649
PHP_MD5Init(&md5ctx);
646-
snprintf(cnonce, sizeof(cnonce), ZEND_LONG_FMT, php_rand());
650+
snprintf(cnonce, sizeof(cnonce), ZEND_LONG_FMT, nonce);
647651
PHP_MD5Update(&md5ctx, (unsigned char*)cnonce, strlen(cnonce));
648652
PHP_MD5Final(hash, &md5ctx);
649653
make_digest(cnonce, hash);

‎ext/spl/php_spl.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@
3939
#include "spl_heap.h"
4040
#include "zend_exceptions.h"
4141
#include "zend_interfaces.h"
42-
#include "ext/standard/php_rand.h"
43-
#include "ext/standard/php_lcg.h"
42+
#include "ext/standard/php_mt_rand.h"
4443
#include "main/snprintf.h"
4544

4645
#ifdef COMPILE_DL_SPL
@@ -747,10 +746,6 @@ PHPAPI zend_string *php_spl_object_hash(zval *obj) /* {{{*/
747746
intptr_t hash_handle, hash_handlers;
748747

749748
if (!SPL_G(hash_mask_init)) {
750-
if (!BG(mt_rand_is_seeded)) {
751-
php_mt_srand((uint32_t)GENERATE_SEED());
752-
}
753-
754749
SPL_G(hash_mask_handle) = (intptr_t)(php_mt_rand() >> 1);
755750
SPL_G(hash_mask_handlers) = (intptr_t)(php_mt_rand() >> 1);
756751
SPL_G(hash_mask_init) = 1;

‎ext/standard/array.c‎

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "php_string.h"
4747
#include "php_rand.h"
4848
#include "zend_smart_str.h"
49+
#include "zend_bitset.h"
4950
#include "ext/spl/spl_array.h"
5051

5152
/* {{{ defines */
@@ -5033,56 +5034,79 @@ PHP_FUNCTION(array_rand)
50335034
{
50345035
zval *input;
50355036
zend_long randval, num_req = 1;
5036-
int num_avail;
50375037
zend_string *string_key;
50385038
zend_ulong num_key;
5039+
int i;
5040+
int num_avail;
5041+
zend_bitset bitset;
5042+
int negative_bitset = 0;
5043+
uint32_t bitset_len;
50395044

50405045
if (zend_parse_parameters(ZEND_NUM_ARGS(), "a|l", &input, &num_req) == FAILURE) {
50415046
return;
50425047
}
50435048

50445049
num_avail = zend_hash_num_elements(Z_ARRVAL_P(input));
50455050

5046-
if (ZEND_NUM_ARGS() > 1) {
5047-
if (num_req <= 0 || num_req > num_avail) {
5048-
php_error_docref(NULL, E_WARNING, "Second argument has to be between 1 and the number of elements in the array");
5049-
return;
5050-
}
5051+
if (num_avail == 0) {
5052+
php_error_docref(NULL, E_WARNING, "Array is empty");
5053+
return;
50515054
}
50525055

5053-
/* Make the return value an array only if we need to pass back more than one result. */
5054-
if (num_req > 1) {
5055-
array_init_size(return_value, (uint32_t)num_req);
5056-
}
5056+
if (num_req == 1) {
5057+
randval = php_mt_rand_range(0, num_avail - 1);
50575058

5058-
/* We can't use zend_hash_index_find() because the array may have string keys or gaps. */
5059-
ZEND_HASH_FOREACH_KEY(Z_ARRVAL_P(input), num_key, string_key) {
5060-
if (!num_req) {
5061-
break;
5062-
}
5063-
5064-
randval = php_rand();
5065-
5066-
if ((double) (randval / (PHP_RAND_MAX + 1.0)) < (double) num_req / (double) num_avail) {
5067-
/* If we are returning a single result, just do it. */
5068-
if (Z_TYPE_P(return_value) != IS_ARRAY) {
5059+
ZEND_HASH_FOREACH_KEY(Z_ARRVAL_P(input), num_key, string_key) {
5060+
if (randval-- == 0) {
50695061
if (string_key) {
50705062
RETURN_STR_COPY(string_key);
50715063
} else {
50725064
RETURN_LONG(num_key);
50735065
}
5066+
}
5067+
} ZEND_HASH_FOREACH_END();
5068+
}
5069+
5070+
if (num_req <= 0 || num_req > num_avail) {
5071+
php_error_docref(NULL, E_WARNING, "Second argument has to be between 1 and the number of elements in the array");
5072+
return;
5073+
}
5074+
5075+
/* Make the return value an array only if we need to pass back more than one result. */
5076+
array_init_size(return_value, (uint32_t)num_req);
5077+
if (num_req > (num_avail >> 1)) {
5078+
negative_bitset = 1;
5079+
num_req = num_avail - num_req;
5080+
}
5081+
5082+
ALLOCA_FLAG(use_heap);
5083+
bitset_len = zend_bitset_len(num_avail);
5084+
bitset = ZEND_BITSET_ALLOCA(bitset_len, use_heap);
5085+
zend_bitset_clear(bitset, bitset_len);
5086+
5087+
i = num_req;
5088+
while (i) {
5089+
randval = php_mt_rand_range(0, num_avail - 1);
5090+
if (!zend_bitset_in(bitset, randval)) {
5091+
zend_bitset_incl(bitset, randval);
5092+
i--;
5093+
}
5094+
}
5095+
5096+
/* We can't use zend_hash_index_find() because the array may have string keys or gaps. */
5097+
i = 0;
5098+
ZEND_HASH_FOREACH_KEY(Z_ARRVAL_P(input), num_key, string_key) {
5099+
if (zend_bitset_in(bitset, i) ^ negative_bitset) {
5100+
if (string_key) {
5101+
add_next_index_str(return_value, zend_string_copy(string_key));
50745102
} else {
5075-
/* Append the result to the return value. */
5076-
if (string_key) {
5077-
add_next_index_str(return_value, zend_string_copy(string_key));
5078-
} else {
5079-
add_next_index_long(return_value, num_key);
5080-
}
5103+
add_next_index_long(return_value, num_key);
50815104
}
5082-
num_req--;
50835105
}
5084-
num_avail--;
5106+
i++;
50855107
} ZEND_HASH_FOREACH_END();
5108+
5109+
free_alloca(bitset, use_heap);
50865110
}
50875111
/* }}} */
50885112

‎ext/standard/basic_functions.c‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "zend_operators.h"
3636
#include "ext/standard/php_dns.h"
3737
#include "ext/standard/php_uuencode.h"
38+
#include "ext/standard/php_mt_rand.h"
3839

3940
#ifdef PHP_WIN32
4041
#include "win32/php_win32_globals.h"
@@ -1883,28 +1884,17 @@ ZEND_BEGIN_ARG_INFO(arginfo_quoted_printable_encode, 0)
18831884
ZEND_ARG_INFO(0, str)
18841885
ZEND_END_ARG_INFO()
18851886
/* }}} */
1886-
/* {{{ rand.c */
1887-
ZEND_BEGIN_ARG_INFO_EX(arginfo_srand, 0, 0, 0)
1888-
ZEND_ARG_INFO(0, seed)
1889-
ZEND_END_ARG_INFO()
1890-
1887+
/* {{{ mt_rand.c */
18911888
ZEND_BEGIN_ARG_INFO_EX(arginfo_mt_srand, 0, 0, 0)
18921889
ZEND_ARG_INFO(0, seed)
1893-
ZEND_END_ARG_INFO()
1894-
1895-
ZEND_BEGIN_ARG_INFO_EX(arginfo_rand, 0, 0, 0)
1896-
ZEND_ARG_INFO(0, min)
1897-
ZEND_ARG_INFO(0, max)
1890+
ZEND_ARG_INFO(0, mode)
18981891
ZEND_END_ARG_INFO()
18991892

19001893
ZEND_BEGIN_ARG_INFO_EX(arginfo_mt_rand, 0, 0, 0)
19011894
ZEND_ARG_INFO(0, min)
19021895
ZEND_ARG_INFO(0, max)
19031896
ZEND_END_ARG_INFO()
19041897

1905-
ZEND_BEGIN_ARG_INFO(arginfo_getrandmax, 0)
1906-
ZEND_END_ARG_INFO()
1907-
19081898
ZEND_BEGIN_ARG_INFO(arginfo_mt_getrandmax, 0)
19091899
ZEND_END_ARG_INFO()
19101900
/* }}} */
@@ -2860,10 +2850,10 @@ const zend_function_entry basic_functions[] = { /* {{{ */
28602850
PHP_FE(proc_nice, arginfo_proc_nice)
28612851
#endif
28622852

2863-
PHP_FE(rand, arginfo_rand)
2864-
PHP_FE(srand, arginfo_srand)
2865-
PHP_FE(getrandmax, arginfo_getrandmax)
2866-
PHP_FE(mt_rand, arginfo_mt_rand)
2853+
PHP_FALIAS(rand, mt_rand, arginfo_mt_rand)
2854+
PHP_FALIAS(srand, mt_srand, arginfo_mt_srand)
2855+
PHP_FALIAS(getrandmax, mt_getrandmax, arginfo_mt_getrandmax)
2856+
PHP_FE(mt_rand, arginfo_mt_rand)
28672857
PHP_FE(mt_srand, arginfo_mt_srand)
28682858
PHP_FE(mt_getrandmax, arginfo_mt_getrandmax)
28692859

@@ -3478,8 +3468,8 @@ static void php_putenv_destructor(zval *zv) /* {{{ */
34783468

34793469
static void basic_globals_ctor(php_basic_globals *basic_globals_p) /* {{{ */
34803470
{
3481-
BG(rand_is_seeded) = 0;
34823471
BG(mt_rand_is_seeded) = 0;
3472+
BG(mt_rand_mode) = MT_RAND_MT19937;
34833473
BG(umask) = -1;
34843474
BG(next) = NULL;
34853475
BG(left) = -1;
@@ -3661,6 +3651,7 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
36613651
BASIC_MINIT_SUBMODULE(standard_filters)
36623652
BASIC_MINIT_SUBMODULE(user_filters)
36633653
BASIC_MINIT_SUBMODULE(password)
3654+
BASIC_MINIT_SUBMODULE(mt_rand)
36643655

36653656
#if defined(HAVE_LOCALECONV) && defined(ZTS)
36663657
BASIC_MINIT_SUBMODULE(localeconv)

‎ext/standard/basic_functions.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,15 +191,13 @@ typedef struct _php_basic_globals {
191191
char *CurrentStatFile, *CurrentLStatFile;
192192
php_stream_statbuf ssb, lssb;
193193

194-
/* rand.c */
194+
/* mt_rand.c */
195195
uint32_t state[MT_N+1]; /* state vector + 1 extra to not violate ANSI C */
196196
uint32_t *next; /* next random value is computed from here */
197197
int left; /* can *next++ this many times before reloading */
198198

199-
unsigned int rand_seed; /* Seed for rand(), in ts version */
200-
201-
zend_bool rand_is_seeded; /* Whether rand() has been seeded */
202199
zend_bool mt_rand_is_seeded; /* Whether mt_rand() has been seeded */
200+
zend_long mt_rand_mode;
203201

204202
/* syslog.c */
205203
char *syslog_device;

‎ext/standard/config.m4‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ PHP_NEW_EXTENSION(standard, array.c base64.c basic_functions.c browscap.c crc32.
557557
cyr_convert.c datetime.c dir.c dl.c dns.c exec.c file.c filestat.c \
558558
flock_compat.c formatted_print.c fsock.c head.c html.c image.c \
559559
info.c iptc.c lcg.c link.c mail.c math.c md5.c metaphone.c \
560-
microtime.c pack.c pageinfo.c quot_print.c rand.c \
560+
microtime.c pack.c pageinfo.c quot_print.c rand.c mt_rand.c \
561561
soundex.c string.c scanf.c syslog.c type.c uniqid.c url.c \
562562
var.c versioning.c assert.c strnatcmp.c levenshtein.c \
563563
incomplete_class.c url_scanner_ex.c ftp_fopen_wrapper.c \

‎ext/standard/config.w32‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ EXTENSION("standard", "array.c base64.c basic_functions.c browscap.c \
1414
cyr_convert.c datetime.c dir.c dl.c dns.c dns_win32.c exec.c \
1515
file.c filestat.c formatted_print.c fsock.c head.c html.c image.c \
1616
info.c iptc.c lcg.c link_win32.c mail.c math.c md5.c metaphone.c microtime.c \
17-
pack.c pageinfo.c quot_print.c rand.c soundex.c \
17+
pack.c pageinfo.c quot_print.c rand.c mt_rand.c soundex.c \
1818
string.c scanf.c syslog.c type.c uniqid.c url.c var.c \
1919
versioning.c assert.c strnatcmp.c levenshtein.c incomplete_class.c \
2020
url_scanner_ex.c ftp_fopen_wrapper.c http_fopen_wrapper.c \

‎ext/standard/crypt.c‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,12 @@
5454
#include <process.h>
5555
#endif
5656

57-
#include "php_lcg.h"
5857
#include "php_crypt.h"
59-
#include "php_rand.h"
58+
#include "php_random.h"
6059

6160
/* sha512 crypt has the maximal salt length of 123 characters */
6261
#define PHP_MAX_SALT_LEN 123
6362

64-
#define PHP_CRYPT_RAND php_rand()
65-
6663
/* Used to check DES salts to ensure that they contain only valid characters */
6764
#define IS_VALID_SALT_CHARACTER(c) (((c) >= '.' && (c) <= '9') || ((c) >= 'A' && (c) <= 'Z') || ((c) >= 'a' && (c) <= 'z'))
6865

@@ -99,11 +96,10 @@ PHP_MSHUTDOWN_FUNCTION(crypt) /* {{{ */
9996

10097
static unsigned char itoa64[] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
10198

102-
static void php_to64(char *s, zend_long v, int n) /* {{{ */
99+
static void php_to64(char *s, int n) /* {{{ */
103100
{
104101
while (--n >= 0) {
105-
*s++ = itoa64[v&0x3f];
106-
v >>= 6;
102+
*s++ = itoa64[*s&0x3f];
107103
}
108104
}
109105
/* }}} */
@@ -266,9 +262,9 @@ PHP_FUNCTION(crypt)
266262

267263
/* The automatic salt generation covers standard DES, md5-crypt and Blowfish (simple) */
268264
if (!*salt) {
269-
strncpy(salt, "$1$", PHP_MAX_SALT_LEN);
270-
php_to64(&salt[3], PHP_CRYPT_RAND, 4);
271-
php_to64(&salt[7], PHP_CRYPT_RAND, 4);
265+
strncpy(salt, "$1$", 3);
266+
php_random_bytes_throw(&salt[3], 8);
267+
php_to64(&salt[3], 8);
272268
strncpy(&salt[11], "$", PHP_MAX_SALT_LEN - 11);
273269
salt_in_len = strlen(salt);
274270
} else {

0 commit comments

Comments
 (0)