GHSA-HMJ8-5XMH-5573: Yamux DATA Frame DoS Patch Review

Vulnerability

GHSA-HMJ8-5XMH-5573 describes an unauthenticated denial of service in py-libp2p's Yamux handling where a peer can announce a DATA frame body length that is never actually delivered, causing the receiver to block indefinitely while waiting on read_exactly(..., length). The commit linked in the code reference shows the vulnerable call sites reading frame bodies directly from the secured connection without any timeout guard, including paths for SYN, ACK, and generic DATA handling in libp2p/stream_muxer/yamux/yamux.py.[source]

The attack is protocol-cheap: a 12-byte Yamux header can claim a massive payload such as 0xFFFFFFFF bytes and then stop transmitting. Because the receiver trusted the announced length and performed an exact read, the connection could become permanently stuck. The included regression tests model both variants: an oversized 4 GB DATA frame header with no body, and a legal-sized DATA frame whose body is withheld.[source] A third-party summary also characterizes the issue as a connection denial of service via oversized DATA frame handling.[source]

# vulnerable pattern from the patch context
syn_payload = await read_exactly(self.secured_conn, length)
ack_payload = await read_exactly(self.secured_conn, length)
await read_exactly(self.secured_conn, length)

Patch

The patch adds two concrete defenses in yamux.py.[source]

1. A flow-control validation rejects oversized DATA frames before any body read occurs. If typ == TYPE_DATA and length > MAX_WINDOW_SIZE, the implementation logs a warning, sets event_shutting_down, invokes _cleanup_on_error(), and breaks the receive loop. This prevents impossible or policy-violating frame sizes from reaching the blocking read path.

2. A new helper _read_data_body() wraps body reads in trio.fail_after(...), using a configurable timeout derived from PY_YAMUX_DATA_READ_TIMEOUT with a default of 60 seconds. The previous direct read_exactly() calls are replaced with this helper for DATA body consumption. If the timeout expires, the outer handler catches trio.TooSlowError, logs the timeout, marks shutdown, cleans up the connection, and exits the loop.

_DEFAULT_DATA_FRAME_READ_TIMEOUT = 60.0

async def _read_data_body(self, length: int) -> bytes:
    with trio.fail_after(_yamux_data_read_timeout()):
        return await read_exactly(self.secured_conn, length)

if typ == TYPE_DATA and length > MAX_WINDOW_SIZE:
    self.event_shutting_down.set()
    await self._cleanup_on_error()
    break

The tests added in tests/core/stream_muxer/test_yamux.py validate both behaviors: immediate teardown on a 4 GB announced DATA frame and timeout-driven teardown when a 1024-byte body is announced but never sent.[source]

Review

Pros

• The fix addresses the actual blocking primitive rather than only the specific 4 GB proof of concept. Oversized DATA frames are rejected up front, and even valid-sized frames can no longer stall the connection forever if the body is withheld.
• The MAX_WINDOW_SIZE check is protocol-aligned. Since Yamux flow control should already bound in-flight DATA, rejecting lengths above that limit is a strong invariant enforcement at the parser boundary.
• The timeout is narrowly scoped to DATA body reads, minimizing behavioral change outside the vulnerable path.
• Error handling is consistent: on both oversize and timeout conditions the code sets shutdown state and runs cleanup, which is appropriate for a framing/protocol violation on a multiplexed connection.
• The regression tests are meaningful and directly model the exploit mechanics described in the advisory, including a bounded wait to ensure the suite fails on hangs instead of deadlocking.

Cons

• The mitigation for withheld but legal-sized bodies is timeout-based, so correctness depends partly on operational tuning. Extremely slow or backpressured environments could see false-positive connection teardown if PY_YAMUX_DATA_READ_TIMEOUT is set too aggressively.
• The patch appears focused on DATA frame body reads only. Based on the provided diff, the new helper is used where DATA payloads are consumed, but the review cannot confirm from the snippet alone whether all frame types with body semantics are equivalently bounded or whether only DATA was exploitable in practice.
• The default 60-second timeout is a policy choice rather than a protocol guarantee. It is reasonable for DoS resistance, but it may need documentation for operators because it changes failure mode from infinite wait to connection abort.

Verdict

Root-cause.

This patch fixes the root cause at the framing layer by eliminating unbounded trust in peer-declared DATA lengths. The oversized-frame guard prevents impossible or abusive lengths from entering the read path, and the bounded read removes the indefinite-block condition even when the announced length is nominally valid but the peer withholds bytes. The added tests demonstrate both exploit classes and confirm teardown behavior. For software engineers, this is a solid, source-grounded remediation with the main tradeoff being timeout tuning rather than residual exposure to the original hang condition.[source] [source]