From 75b7ffa9b2b947083ecd8e3c0a6782370fdefada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9F=A9=E5=A4=A9=E5=B3=B0-Rango?= Date: Wed, 18 Aug 2021 18:48:22 +0800 Subject: [PATCH] Fix memory invalid read/write (#4379) * Fix ipv6 address buffer overflow * Fix Socket::readVectorAll invalid write * Fix System::gethostbyname with c-ares invalid write. --- ext-src/swoole_async_coro.cc | 2 +- ext-src/swoole_socket_coro.cc | 54 +++++++++++++++++++---------------- src/network/dns.cc | 9 +++++- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/ext-src/swoole_async_coro.cc b/ext-src/swoole_async_coro.cc index 777fe5ff748..a1350c2e470 100644 --- a/ext-src/swoole_async_coro.cc +++ b/ext-src/swoole_async_coro.cc @@ -29,7 +29,7 @@ using swoole::Timer; using swoole::coroutine::Socket; struct DNSCacheEntity { - char address[16]; + char address[INET6_ADDRSTRLEN]; time_t update_time; }; diff --git a/ext-src/swoole_socket_coro.cc b/ext-src/swoole_socket_coro.cc index d5afe6a7cd5..dbbd7e03575 100644 --- a/ext-src/swoole_socket_coro.cc +++ b/ext-src/swoole_socket_coro.cc @@ -843,9 +843,12 @@ static void sw_inline php_swoole_init_socket(zval *zobject, SocketObject *sock) sock->socket->set_zero_copy(true); sock->socket->set_buffer_allocator(sw_zend_string_allocator()); zend_update_property_long(swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("fd"), sock->socket->get_fd()); - zend_update_property_long(swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("domain"), sock->socket->get_sock_domain()); - zend_update_property_long(swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("type"), sock->socket->get_sock_type()); - zend_update_property_long(swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("protocol"), sock->socket->get_sock_protocol()); + zend_update_property_long( + swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("domain"), sock->socket->get_sock_domain()); + zend_update_property_long( + swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("type"), sock->socket->get_sock_type()); + zend_update_property_long( + swoole_socket_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("protocol"), sock->socket->get_sock_protocol()); } SW_API bool php_swoole_export_socket(zval *zobject, Socket *_socket) { @@ -1479,26 +1482,27 @@ static void swoole_socket_coro_read_vector(INTERNAL_FUNCTION_PARAMETERS, const b std::unique_ptr iov(new iovec[iovcnt]); - SW_HASHTABLE_FOREACH_START(vht, zelement) - if (!ZVAL_IS_LONG(zelement)) { - zend_throw_exception_ex(swoole_socket_coro_exception_ce, - EINVAL, - "Item #[%d] must be of type int, %s given", - iov_index, - zend_get_type_by_const(Z_TYPE_P(zelement))); - RETURN_FALSE; - } - if (Z_LVAL_P(zelement) < 0) { - zend_throw_exception_ex( - swoole_socket_coro_exception_ce, EINVAL, "Item #[%d] must be greater than 0", iov_index); - RETURN_FALSE; - } - size_t iov_len = Z_LVAL_P(zelement); + SW_HASHTABLE_FOREACH_START(vht, zelement) { + if (!ZVAL_IS_LONG(zelement)) { + zend_throw_exception_ex(swoole_socket_coro_exception_ce, + EINVAL, + "Item #[%d] must be of type int, %s given", + iov_index, + zend_get_type_by_const(Z_TYPE_P(zelement))); + RETURN_FALSE; + } + if (Z_LVAL_P(zelement) < 0) { + zend_throw_exception_ex( + swoole_socket_coro_exception_ce, EINVAL, "Item #[%d] must be greater than 0", iov_index); + RETURN_FALSE; + } + size_t iov_len = Z_LVAL_P(zelement); - iov[iov_index].iov_base = zend_string_alloc(iov_len, 0)->val; - iov[iov_index].iov_len = iov_len; - iov_index++; - total_length += iov_len; + iov[iov_index].iov_base = zend_string_alloc(iov_len, 0)->val; + iov[iov_index].iov_len = iov_len; + iov_index++; + total_length += iov_len; + } SW_HASHTABLE_FOREACH_END(); swoole::network::IOVector io_vector((struct iovec *) iov.get(), iovcnt); @@ -1533,6 +1537,7 @@ static void swoole_socket_coro_read_vector(INTERNAL_FUNCTION_PARAMETERS, const b real_count = iov_index + 1; zend_string *str = zend::fetch_zend_string_by_val((char *) iov[iov_index].iov_base); iov[iov_index].iov_base = sw_zend_string_recycle(str, iov[iov_index].iov_len, offset_bytes)->val; + iov[iov_index].iov_len = offset_bytes; free_func(iov.get(), iovcnt, real_count); } else { real_count = iovcnt; @@ -1767,9 +1772,10 @@ static PHP_METHOD(swoole_socket_coro, getOption) { } case SO_RCVTIMEO: case SO_SNDTIMEO: { - double timeout = sock->socket->get_timeout(optname == SO_RCVTIMEO ? Socket::TIMEOUT_READ : Socket::TIMEOUT_WRITE); + double timeout = + sock->socket->get_timeout(optname == SO_RCVTIMEO ? Socket::TIMEOUT_READ : Socket::TIMEOUT_WRITE); array_init(return_value); - int sec = (int) timeout; + int sec = (int) timeout; add_assoc_long(return_value, "sec", (int) timeout); add_assoc_long(return_value, "usec", (timeout - (double) sec) * 1000000); break; diff --git a/src/network/dns.cc b/src/network/dns.cc index 3678c9c36ee..da5531bf269 100644 --- a/src/network/dns.cc +++ b/src/network/dns.cc @@ -402,6 +402,7 @@ struct ResolvContext { int error; bool completed; Coroutine *co; + std::shared_ptr defer_task_cancelled; std::unordered_map sockets; std::vector result; }; @@ -428,6 +429,7 @@ std::vector dns_lookup_impl_with_cares(const char *domain, int fami Coroutine *co = Coroutine::get_current_safe(); ctx.co = co; ctx.completed = false; + ctx.defer_task_cancelled = std::make_shared(false); char lookups[] = "fb"; int res; ctx.ares_opts.lookups = lookups; @@ -533,8 +535,12 @@ std::vector dns_lookup_impl_with_cares(const char *domain, int fami } _resume: if (ctx->co && ctx->co->is_suspending()) { + auto _cancelled = ctx->defer_task_cancelled; swoole_event_defer( - [](void *data) { + [_cancelled](void *data) { + if (*_cancelled) { + return; + } Coroutine *co = reinterpret_cast(data); co->resume(); }, @@ -573,6 +579,7 @@ std::vector dns_lookup_impl_with_cares(const char *domain, int fami break; } } + *ctx.defer_task_cancelled = true; ares_destroy(ctx.channel); _return: return ctx.result;