Skip to content

fix(websocket): enforce MAX_PAYLOAD_LENGTH cap on message accumulation - #875

Open
krleejihyeong wants to merge 1 commit into
ithewei:masterfrom
krleejihyeong:fix/websocket-payload-cap
Open

fix(websocket): enforce MAX_PAYLOAD_LENGTH cap on message accumulation#875
krleejihyeong wants to merge 1 commit into
ithewei:masterfrom
krleejihyeong:fix/websocket-payload-cap

Conversation

@krleejihyeong

Copy link
Copy Markdown
Contributor

Summary

In WebSocketParser.cpp, on_frame_body() appends every received frame's
payload into wp->message with no upper bound. The MAX_PAYLOAD_LENGTH
(16MB) constant is defined but only used as a reserve() hint in
on_frame_header() — it never actually caps accumulation. In addition,
on_frame_header() only clears wp->message when the state is
WS_FRAME_BEGIN or WS_FRAME_FIN, so a stream of FIN=0 (fragmented)
frames never triggers a clear and keeps accumulating indefinitely.

Since the WebSocket handshake is an unauthenticated HTTP Upgrade, an
unauthenticated remote attacker can keep a single connection open and
keep sending non-FIN frames to grow the server's memory usage without
bound.

I reproduced this locally: sending 40 x 1MB non-FIN frames over a single
connection grew the server process RSS from a baseline of 4,352 kB to
76,956 kB (+72MB) and counting.

Root cause

static int on_frame_body(websocket_parser* parser, const char* at, size_t length) {
    WebSocketParser* wp = (WebSocketParser*)parser->data;
    if (wp->parser->flags & WS_HAS_MASK)
        websocket_parser_decode((char*)at, at, length, wp->parser);
    wp->message.append(at, length);   // no size limit
    return 0;
}

Fix

Added a check in on_frame_body(): if the incoming chunk itself, or the
accumulated message size after appending it, would exceed
MAX_PAYLOAD_LENGTH, the parser callback returns non-zero. This stops
websocket_parser_execute and the caller (HttpHandler::FeedRecvData)
then closes the connection.

Testing

  • Before patch: sending non-FIN fragments repeatedly grew server RSS
    without bound (+72MB and climbing in my test)
  • After patch: the same scenario causes the connection to be closed once
    the accumulated size would exceed the cap; server RSS stays at
    baseline (~4,396 kB) and the process keeps running normally afterward
  • Normal single-frame and fragmented+FIN message echo still works
    correctly after the patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant