From d75c1d00a98faedbc36031ab42780e63192d9e2b Mon Sep 17 00:00:00 2001 From: nielsdos <7771979+nielsdos@users.noreply.github.com> Date: Tue, 2 May 2023 23:45:50 +0200 Subject: [PATCH] Fix GH-11175 and GH-11177: Stream socket timeout undefined behaviour A negative value like -1 may overflow and cause incorrect results in the timeout variable, which causes an immediate timeout. As this is caused by undefined behaviour the exact behaviour depends on the compiler, its version, and the platform. A large overflow is also possible, if an extremely large timeout value is passed we also set an indefinite timeout. This is because the timeout value is at least a 64-bit number and waiting for UINT64_MAX/1000000 seconds is waiting about 584K years. Closes GH-11183. --- NEWS | 5 +++++ ext/standard/streamsfuncs.c | 39 +++++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index 4c29303a5ad..2f9d717c0d2 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,11 @@ PHP NEWS . Fixed bug GH-11138 (move_uploaded_file() emits open_basedir warning for source file). (ilutov) +- Streams: + . Fixed bug GH-11175 (Stream Socket Timeout). (nielsdos) + . Fixed bug GH-11177 (ASAN UndefinedBehaviorSanitizer when timeout = -1 + passed to stream_socket_accept/stream_socket_client). (nielsdos) + 11 May 2023, PHP 8.1.19 - Core: diff --git a/ext/standard/streamsfuncs.c b/ext/standard/streamsfuncs.c index 188bcee8854..8bd345cc2c1 100644 --- a/ext/standard/streamsfuncs.c +++ b/ext/standard/streamsfuncs.c @@ -33,11 +33,13 @@ #ifndef PHP_WIN32 #define php_select(m, r, w, e, t) select(m, r, w, e, t) typedef unsigned long long php_timeout_ull; +#define PHP_TIMEOUT_ULL_MAX ULLONG_MAX #else #include "win32/select.h" #include "win32/sockets.h" #include "win32/console.h" typedef unsigned __int64 php_timeout_ull; +#define PHP_TIMEOUT_ULL_MAX UINT64_MAX #endif #define GET_CTX_OPT(stream, wrapper, name, val) (PHP_STREAM_CONTEXT(stream) && NULL != (val = php_stream_context_get_option(PHP_STREAM_CONTEXT(stream), wrapper, name))) @@ -134,14 +136,21 @@ PHP_FUNCTION(stream_socket_client) } /* prepare the timeout value for use */ - conv = (php_timeout_ull) (timeout * 1000000.0); + struct timeval *tv_pointer; + if (timeout < 0.0 || timeout >= (double) PHP_TIMEOUT_ULL_MAX / 1000000.0) { + tv_pointer = NULL; + } else { + conv = (php_timeout_ull) (timeout * 1000000.0); #ifdef PHP_WIN32 - tv.tv_sec = (long)(conv / 1000000); - tv.tv_usec =(long)(conv % 1000000); + tv.tv_sec = (long)(conv / 1000000); + tv.tv_usec = (long)(conv % 1000000); #else - tv.tv_sec = conv / 1000000; - tv.tv_usec = conv % 1000000; + tv.tv_sec = conv / 1000000; + tv.tv_usec = conv % 1000000; #endif + tv_pointer = &tv; + } + if (zerrno) { ZEND_TRY_ASSIGN_REF_LONG(zerrno, 0); } @@ -152,7 +161,7 @@ PHP_FUNCTION(stream_socket_client) stream = php_stream_xport_create(ZSTR_VAL(host), ZSTR_LEN(host), REPORT_ERRORS, STREAM_XPORT_CLIENT | (flags & PHP_STREAM_CLIENT_CONNECT ? STREAM_XPORT_CONNECT : 0) | (flags & PHP_STREAM_CLIENT_ASYNC_CONNECT ? STREAM_XPORT_CONNECT_ASYNC : 0), - hashkey, &tv, context, &errstr, &err); + hashkey, tv_pointer, context, &errstr, &err); if (stream == NULL) { @@ -275,19 +284,25 @@ PHP_FUNCTION(stream_socket_accept) php_stream_from_zval(stream, zstream); /* prepare the timeout value for use */ - conv = (php_timeout_ull) (timeout * 1000000.0); + struct timeval *tv_pointer; + if (timeout < 0.0 || timeout >= (double) PHP_TIMEOUT_ULL_MAX / 1000000.0) { + tv_pointer = NULL; + } else { + conv = (php_timeout_ull) (timeout * 1000000.0); #ifdef PHP_WIN32 - tv.tv_sec = (long)(conv / 1000000); - tv.tv_usec = (long)(conv % 1000000); + tv.tv_sec = (long)(conv / 1000000); + tv.tv_usec = (long)(conv % 1000000); #else - tv.tv_sec = conv / 1000000; - tv.tv_usec = conv % 1000000; + tv.tv_sec = conv / 1000000; + tv.tv_usec = conv % 1000000; #endif + tv_pointer = &tv; + } if (0 == php_stream_xport_accept(stream, &clistream, zpeername ? &peername : NULL, NULL, NULL, - &tv, &errstr + tv_pointer, &errstr ) && clistream) { if (peername) {