Skip to content

v2026.06.01.00

v2026.06.01.00: closeNow() before socket_.reset() to fix Fizz handshake UAF
@jianfeng-tang jianfeng-tang tagged this 29 May 04:41 
Summary:
fizz::client::AsyncFizzClient stores its ConnectCallback as a raw
pointer and arms a handshake-timeout timer when connect() is called.
On the inline-Fizz outbound path in mcrouter, that pointer is the
AsyncMcClientImpl. If the underlying McFizzClient transport outlives
AsyncMcClientImpl with the timer still armed, deliverHandshakeError
dereferences a freed ConnectCallback and crashes. The race window
scales with the configured connect/handshake timeout.

Two paths in AsyncMcClientImpl destroy the transport via
socket_.reset() without first calling socket_->closeNow():

  - connectErr()
  - processShutdown()

closeNow() runs deliverAllErrors -> deliverHandshakeError synchronously,
which cancels the Fizz handshake-timeout timer, clears the raw
ConnectCallback pointer, and delivers a terminal connectErr(). After
closeNow(), even if a DestructorGuard keeps the transport alive past
socket_.reset(), the dangling-pointer hazard is gone.

This matches the existing pattern in ~AsyncMcClientImpl(). Both
reset()-without-closeNow() sites now follow it.

Out of scope: the async SSL handshake-offload path (already anchored
via selfPtr_.lock()), plaintext / non-Fizz, and inbound (server-side)
Fizz handshakes (already fixed separately).

Reviewed By: knekritz

Differential Revision: D105440074

fbshipit-source-id: 4be476feab9041e900f93db690a32d3ac7d8e744
Assets 2
Loading