From b7b2409f73586d09a2223b5857261ee16d0fb667 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 19 May 2025 21:29:00 +0200 Subject: [PATCH 1/2] Fix missing checks against php_set_blocking() in xp_ssl.c --- ext/openssl/xp_ssl.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 67846ad3aae83..5617d06759d7b 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -2092,8 +2092,9 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) { /* If the socket was originally blocking, set it back. */ if (began_blocked) { - php_set_sock_blocking(sslsock->s.socket, 1); - sslsock->s.is_blocked = 1; + if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { + sslsock->s.is_blocked = 1; + } } sslsock->s.timeout_event = 1; return -1; @@ -2520,8 +2521,9 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) { /* If the socket was originally blocking, set it back. */ if (began_blocked) { - php_set_sock_blocking(sslsock->s.socket, 1); - sslsock->s.is_blocked = 1; + if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { + sslsock->s.is_blocked = 1; + } } sslsock->s.timeout_event = 1; return PHP_STREAM_OPTION_RETURN_ERR; @@ -2572,8 +2574,9 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val if (began_blocked && !sslsock->s.is_blocked) { // Set it back to blocking - php_set_sock_blocking(sslsock->s.socket, 1); - sslsock->s.is_blocked = 1; + if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { + sslsock->s.is_blocked = 1; + } } } else { #ifdef PHP_WIN32 From 19a56bfcfad69af5464aa1b3d93bfea72f25160b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 19 May 2025 21:59:07 +0200 Subject: [PATCH 2/2] Split off php_set_sock_blocking() and s.is_blocked to a separate function This makes it harder to forget the check and keeps the variable and function call consistent. --- ext/openssl/xp_ssl.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 5617d06759d7b..1c7aabefa1bcd 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -1901,6 +1901,15 @@ static int php_openssl_capture_peer_certs(php_stream *stream, } /* }}} */ +static zend_result php_openssl_set_blocking(php_openssl_netstream_data_t *sslsock, int block) +{ + zend_result result = php_set_sock_blocking(sslsock->s.socket, block); + if (EXPECTED(SUCCESS == result)) { + sslsock->s.is_blocked = block; + } + return result; +} + static int php_openssl_enable_crypto(php_stream *stream, php_openssl_netstream_data_t *sslsock, php_stream_xport_crypto_param *cparam) /* {{{ */ @@ -1929,8 +1938,7 @@ static int php_openssl_enable_crypto(php_stream *stream, sslsock->state_set = 1; } - if (SUCCESS == php_set_sock_blocking(sslsock->s.socket, 0)) { - sslsock->s.is_blocked = 0; + if (SUCCESS == php_openssl_set_blocking(sslsock, 0)) { /* The following mode are added only if we are able to change socket * to non blocking mode which is also used for read and write */ SSL_set_mode(sslsock->ssl_handle, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); @@ -1983,8 +1991,8 @@ static int php_openssl_enable_crypto(php_stream *stream, } } while (retry); - if (sslsock->s.is_blocked != blocked && SUCCESS == php_set_sock_blocking(sslsock->s.socket, blocked)) { - sslsock->s.is_blocked = blocked; + if (sslsock->s.is_blocked != blocked) { + php_openssl_set_blocking(sslsock, blocked); } if (n == 1) { @@ -2067,8 +2075,8 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si timeout = &sslsock->s.timeout; } - if (timeout && php_set_sock_blocking(sslsock->s.socket, 0) == SUCCESS) { - sslsock->s.is_blocked = 0; + if (timeout) { + php_openssl_set_blocking(sslsock, 0); } if (!sslsock->s.is_blocked && timeout && (timeout->tv_sec > 0 || (timeout->tv_sec == 0 && timeout->tv_usec))) { @@ -2092,9 +2100,7 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) { /* If the socket was originally blocking, set it back. */ if (began_blocked) { - if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { - sslsock->s.is_blocked = 1; - } + php_openssl_set_blocking(sslsock, 1); } sslsock->s.timeout_event = 1; return -1; @@ -2189,8 +2195,8 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si } /* And if we were originally supposed to be blocking, let's reset the socket to that. */ - if (began_blocked && php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { - sslsock->s.is_blocked = 1; + if (began_blocked) { + php_openssl_set_blocking(sslsock, 1); } return 0 > nr_bytes ? 0 : nr_bytes; @@ -2496,8 +2502,8 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val timeout = &tv; } - if (timeout && php_set_sock_blocking(sslsock->s.socket, 0) == SUCCESS) { - sslsock->s.is_blocked = 0; + if (timeout) { + php_openssl_set_blocking(sslsock, 0); } if (!sslsock->s.is_blocked && timeout && (timeout->tv_sec > 0 || (timeout->tv_sec == 0 && timeout->tv_usec))) { @@ -2521,9 +2527,7 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) { /* If the socket was originally blocking, set it back. */ if (began_blocked) { - if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { - sslsock->s.is_blocked = 1; - } + php_openssl_set_blocking(sslsock, 1); } sslsock->s.timeout_event = 1; return PHP_STREAM_OPTION_RETURN_ERR; @@ -2574,9 +2578,7 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val if (began_blocked && !sslsock->s.is_blocked) { // Set it back to blocking - if (php_set_sock_blocking(sslsock->s.socket, 1) == SUCCESS) { - sslsock->s.is_blocked = 1; - } + php_openssl_set_blocking(sslsock, 1); } } else { #ifdef PHP_WIN32