From a1544a5ea4bea2c79277e4ffa7ca4968548bb90b Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 5 Jul 2025 14:31:43 +0100 Subject: [PATCH 1/3] ext/sockets: socket_addrinfo_lookup narrowing down socket family check. --- ext/sockets/sockets.c | 8 ++++++-- ext/sockets/tests/socket_getaddrinfo_error.phpt | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 82634cc8f3e54..2673bc67e5391 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -2774,8 +2774,11 @@ PHP_FUNCTION(socket_addrinfo_lookup) zend_argument_type_error(3, "\"ai_family\" key must be of type int, %s given", zend_zval_type_name(hint)); RETURN_THROWS(); } - if (val < 0 || val >= AF_MAX) { - zend_argument_value_error(3, "\"ai_family\" key must be between 0 and %d", AF_MAX - 1); + // Some platforms support also PF_LOCAL/AF_UNIX (e.g. FreeBSD) but the security concerns implied + // make it not worth handling it (e.g. unwarranted write permissions on the socket). + // Note existing socket_addrinfo* api already forbid such case. + if (val != AF_INET && val != AF_INET6) { + zend_argument_value_error(3, "\"ai_family\" key must be AF_INET or AF_INET6"); RETURN_THROWS(); } hints.ai_family = (int)val; @@ -2856,6 +2859,7 @@ PHP_FUNCTION(socket_addrinfo_bind) php_sock->blocking = 1; switch(php_sock->type) { + // ZEND_ASSERT ? Addrinfo being opaque read-only, should not happen with previous change case AF_UNIX: { // AF_UNIX sockets via getaddrino are not implemented due to security problems diff --git a/ext/sockets/tests/socket_getaddrinfo_error.phpt b/ext/sockets/tests/socket_getaddrinfo_error.phpt index c3488b540c69b..0f91792fbf9a7 100644 --- a/ext/sockets/tests/socket_getaddrinfo_error.phpt +++ b/ext/sockets/tests/socket_getaddrinfo_error.phpt @@ -110,7 +110,7 @@ socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be of type i socket_addrinfo_lookup(): Argument #3 ($hints) "ai_socktype" key must be of type int, stdClass given socket_addrinfo_lookup(): Argument #3 ($hints) "ai_flags" key must be of type int, stdClass given socket_addrinfo_lookup(): Argument #3 ($hints) "ai_protocol" key must be of type int, stdClass given -socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be between 0 and %d +socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be AF_INET or AF_INET6 socket_addrinfo_lookup(): Argument #3 ($hints) "ai_socktype" key must be between 0 and %d socket_addrinfo_lookup(): Argument #3 ($hints) "ai_flags" key must be between 0 and %d socket_addrinfo_lookup(): Argument #3 ($hints) "ai_protocol" key must be between 0 and %d From 8ac351375b0540f14d449f2e112b1a98d53ade4b Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 5 Jul 2025 15:04:27 +0100 Subject: [PATCH 2/3] simplifying socket_addrinfo_bind/connect as we can assume the family is correct. --- ext/sockets/sockets.c | 59 +++++-------------------------------------- 1 file changed, 6 insertions(+), 53 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 2673bc67e5391..3750bcd90b7d9 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -2830,7 +2830,6 @@ PHP_FUNCTION(socket_addrinfo_lookup) PHP_FUNCTION(socket_addrinfo_bind) { zval *arg1; - int retval; php_addrinfo *ai; php_socket *php_sock; @@ -2840,6 +2839,8 @@ PHP_FUNCTION(socket_addrinfo_bind) ai = Z_ADDRESS_INFO_P(arg1); + ZEND_ASSERT(ai->addrinfo.ai_family == AF_INET || ai->addrinfo.ai_family == AF_INET6); + PHP_ETH_PROTO_CHECK(ai->addrinfo.ai_protocol, ai->addrinfo.ai_family); object_init_ex(return_value, socket_ce); @@ -2858,32 +2859,7 @@ PHP_FUNCTION(socket_addrinfo_bind) php_sock->error = 0; php_sock->blocking = 1; - switch(php_sock->type) { - // ZEND_ASSERT ? Addrinfo being opaque read-only, should not happen with previous change - case AF_UNIX: - { - // AF_UNIX sockets via getaddrino are not implemented due to security problems - close(php_sock->bsd_socket); - zval_ptr_dtor(return_value); - RETURN_FALSE; - } - - case AF_INET: -#ifdef HAVE_IPV6 - case AF_INET6: -#endif - { - retval = bind(php_sock->bsd_socket, ai->addrinfo.ai_addr, ai->addrinfo.ai_addrlen); - break; - } - default: - close(php_sock->bsd_socket); - zval_ptr_dtor(return_value); - zend_argument_value_error(1, "must be one of AF_UNIX, AF_INET, or AF_INET6"); - RETURN_THROWS(); - } - - if (retval != 0) { + if (bind(php_sock->bsd_socket, ai->addrinfo.ai_addr, ai->addrinfo.ai_addrlen) != 0) { PHP_SOCKET_ERROR(php_sock, "Unable to bind address", errno); close(php_sock->bsd_socket); zval_ptr_dtor(return_value); @@ -2896,7 +2872,6 @@ PHP_FUNCTION(socket_addrinfo_bind) PHP_FUNCTION(socket_addrinfo_connect) { zval *arg1; - int retval; php_addrinfo *ai; php_socket *php_sock; @@ -2906,6 +2881,8 @@ PHP_FUNCTION(socket_addrinfo_connect) ai = Z_ADDRESS_INFO_P(arg1); + ZEND_ASSERT(ai->addrinfo.ai_family == AF_INET || ai->addrinfo.ai_family == AF_INET6); + PHP_ETH_PROTO_CHECK(ai->addrinfo.ai_protocol, ai->addrinfo.ai_family); object_init_ex(return_value, socket_ce); @@ -2924,31 +2901,7 @@ PHP_FUNCTION(socket_addrinfo_connect) php_sock->error = 0; php_sock->blocking = 1; - switch(php_sock->type) { - case AF_UNIX: - { - // AF_UNIX sockets via getaddrino are not implemented due to security problems - close(php_sock->bsd_socket); - zval_ptr_dtor(return_value); - RETURN_FALSE; - } - - case AF_INET: -#ifdef HAVE_IPV6 - case AF_INET6: -#endif - { - retval = connect(php_sock->bsd_socket, ai->addrinfo.ai_addr, ai->addrinfo.ai_addrlen); - break; - } - default: - zend_argument_value_error(1, "socket type must be one of AF_UNIX, AF_INET, or AF_INET6"); - close(php_sock->bsd_socket); - zval_ptr_dtor(return_value); - RETURN_THROWS(); - } - - if (retval != 0) { + if (connect(php_sock->bsd_socket, ai->addrinfo.ai_addr, ai->addrinfo.ai_addrlen) != 0) { PHP_SOCKET_ERROR(php_sock, "Unable to connect address", errno); close(php_sock->bsd_socket); zval_ptr_dtor(return_value); From 242f5d352212d63fa26ce6478749d5beb59993c1 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 5 Jul 2025 15:34:41 +0100 Subject: [PATCH 3/3] ipv6 or not ipv6.. --- ext/sockets/sockets.c | 5 +++++ ext/sockets/tests/socket_getaddrinfo_error.phpt | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 3750bcd90b7d9..2f2eb3465fdeb 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -2777,8 +2777,13 @@ PHP_FUNCTION(socket_addrinfo_lookup) // Some platforms support also PF_LOCAL/AF_UNIX (e.g. FreeBSD) but the security concerns implied // make it not worth handling it (e.g. unwarranted write permissions on the socket). // Note existing socket_addrinfo* api already forbid such case. +#ifdef HAVE_IPV6 if (val != AF_INET && val != AF_INET6) { zend_argument_value_error(3, "\"ai_family\" key must be AF_INET or AF_INET6"); +#else + if (val != AF_INET) { + zend_argument_value_error(3, "\"ai_family\" key must be AF_INET"); +#endif RETURN_THROWS(); } hints.ai_family = (int)val; diff --git a/ext/sockets/tests/socket_getaddrinfo_error.phpt b/ext/sockets/tests/socket_getaddrinfo_error.phpt index 0f91792fbf9a7..a2455821e70e2 100644 --- a/ext/sockets/tests/socket_getaddrinfo_error.phpt +++ b/ext/sockets/tests/socket_getaddrinfo_error.phpt @@ -110,7 +110,7 @@ socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be of type i socket_addrinfo_lookup(): Argument #3 ($hints) "ai_socktype" key must be of type int, stdClass given socket_addrinfo_lookup(): Argument #3 ($hints) "ai_flags" key must be of type int, stdClass given socket_addrinfo_lookup(): Argument #3 ($hints) "ai_protocol" key must be of type int, stdClass given -socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be AF_INET or AF_INET6 +socket_addrinfo_lookup(): Argument #3 ($hints) "ai_family" key must be AF_INET%A socket_addrinfo_lookup(): Argument #3 ($hints) "ai_socktype" key must be between 0 and %d socket_addrinfo_lookup(): Argument #3 ($hints) "ai_flags" key must be between 0 and %d socket_addrinfo_lookup(): Argument #3 ($hints) "ai_protocol" key must be between 0 and %d