From 68ec97ba59eeff12b47dfd14a306d172da4e2355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 21 Jan 2023 01:24:04 +0100 Subject: [PATCH] session: Remove PS_EXTRA_RAND_BYTES MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was introduced in 3467526a65bfb15eaf9ec49a0b5673b84e26bca4 and the corresponding RFC gives some reasoning. However the CSPRNG being “not secure enough” is not a thing and reading these extra bytes is just security theater: If the CSPRNG would hypothetically be broken, then PHP’s session IDs are the least of one’s concerns, because we already trust it in `random_bytes()` and might generate long-term secrets using that. --- ext/session/session.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ext/session/session.c b/ext/session/session.c index 4bfc973ed537b..7f830ff82d24c 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -306,17 +306,14 @@ static void bin_to_readable(unsigned char *in, size_t inlen, char *out, size_t o } /* }}} */ -#define PS_EXTRA_RAND_BYTES 60 - PHPAPI zend_string *php_session_create_id(PS_CREATE_SID_ARGS) /* {{{ */ { - unsigned char rbuf[PS_MAX_SID_LENGTH + PS_EXTRA_RAND_BYTES]; + unsigned char rbuf[PS_MAX_SID_LENGTH]; zend_string *outid; /* It would be enough to read ceil(sid_length * sid_bits_per_character / 8) bytes here. * We read sid_length bytes instead for simplicity. */ - /* Read additional PS_EXTRA_RAND_BYTES just in case CSPRNG is not safe enough */ - if (php_random_bytes_throw(rbuf, PS(sid_length) + PS_EXTRA_RAND_BYTES) == FAILURE) { + if (php_random_bytes_throw(rbuf, PS(sid_length)) == FAILURE) { return NULL; }