From 884dab4a6a98bc125976d3dd7ed8bf2ea9b3f90b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 10 Apr 2026 21:12:51 -1000 Subject: [PATCH] [socket] Null cached_sock_ in close() to prevent post-close UAF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore the cached_sock_ = nullptr assignment inside close() on the fast-select path. The lwip slot can be recycled for a new connection as soon as the underlying close() returns, so any dereference of cached_sock_ afterwards would touch an unrelated socket's pcb. No current caller does this — setsockopt(TCP_NODELAY) and ready() are the only consumers and neither is invoked post-close today — but leaving the pointer dangling is a footgun for future changes. The fd_ = -1 sentinel alone would catch the ready() path via closed semantics, but setsockopt() reaches cached_sock_ directly and would not be protected. Null the pointer so the protection is by construction rather than by caller discipline. --- esphome/components/socket/bsd_sockets_impl.cpp | 11 ++++++++--- esphome/components/socket/lwip_sockets_impl.cpp | 11 ++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/esphome/components/socket/bsd_sockets_impl.cpp b/esphome/components/socket/bsd_sockets_impl.cpp index baf030f2b1..b73440ef55 100644 --- a/esphome/components/socket/bsd_sockets_impl.cpp +++ b/esphome/components/socket/bsd_sockets_impl.cpp @@ -27,9 +27,14 @@ int BSDSocketImpl::close() { // Already closed, or never opened. return 0; } -#ifndef USE_LWIP_FAST_SELECT - // On the fast-select path there is no per-socket unhook needed — all LwIP sockets - // share the same static event_callback. +#ifdef USE_LWIP_FAST_SELECT + // Null the cached lwip_sock pointer before closing. The underlying lwip slot can be + // recycled for a new connection as soon as ::close() returns, so anything that might + // dereference cached_sock_ post-close (e.g. setsockopt(TCP_NODELAY)) would otherwise + // touch an unrelated socket's pcb. No per-socket callback unhook is needed — + // all LwIP sockets share the same static event_callback. + this->cached_sock_ = nullptr; +#else if (this->loop_monitored_) { App.unregister_socket_fd(this->fd_); } diff --git a/esphome/components/socket/lwip_sockets_impl.cpp b/esphome/components/socket/lwip_sockets_impl.cpp index 52c94bc7f6..618d17aabe 100644 --- a/esphome/components/socket/lwip_sockets_impl.cpp +++ b/esphome/components/socket/lwip_sockets_impl.cpp @@ -27,9 +27,14 @@ int LwIPSocketImpl::close() { // Already closed, or never opened. return 0; } -#ifndef USE_LWIP_FAST_SELECT - // On the fast-select path there is no per-socket unhook needed — all LwIP sockets - // share the same static event_callback. +#ifdef USE_LWIP_FAST_SELECT + // Null the cached lwip_sock pointer before closing. The underlying lwip slot can be + // recycled for a new connection as soon as lwip_close() returns, so anything that + // might dereference cached_sock_ post-close (e.g. setsockopt(TCP_NODELAY)) would + // otherwise touch an unrelated socket's pcb. No per-socket callback unhook is needed — + // all LwIP sockets share the same static event_callback. + this->cached_sock_ = nullptr; +#else if (this->loop_monitored_) { App.unregister_socket_fd(this->fd_); }