Skip to content

Commit

Permalink
Fix GH-11175 and GH-11177: Stream socket timeout undefined behaviour
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nielsdos committed May 3, 2023
1 parent 4ca8daf commit d75c1d0
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 12 deletions.
5 changes: 5 additions & 0 deletions NEWS
Expand Up @@ -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:
Expand Down
39 changes: 27 additions & 12 deletions ext/standard/streamsfuncs.c
Expand Up @@ -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)))
Expand Down Expand Up @@ -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);
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d75c1d0

Please sign in to comment.