Skip to content

Three robustness/security fixes in the frame path #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcocarnut wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Three robustness/security fixes in the frame path #120

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
384d96b
Fix off-by-one out-of-bounds write on a 125-byte control frame
marcocarnut Sep 16, 2026
bc928f3
Reject unmasked client frames (RFC 6455 section 5.1)
marcocarnut Sep 16, 2026
55a0cfa
Bound the handshake read with SO_RCVTIMEO (slowloris mitigation)
marcocarnut Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion src/ws.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,13 @@ struct ws_frame_data
unsigned char *msg;
/**
* @brief Control frame payload
*
* Sized 125 (the RFC 6455 maximum control-frame payload) + 1 for the
* terminating NUL that read_single_frame() writes at msg[*msg_idx] on a
* FIN frame. A control frame may carry a full 125-byte payload, after
* which *msg_idx == 125, so the buffer must have room for index 125.
*/
unsigned char msg_ctrl[125];
unsigned char msg_ctrl[125 + 1];
/**
* @brief Current byte position.
*/
Expand Down Expand Up @@ -1686,6 +1691,20 @@ static int next_complete_frame(struct ws_frame_data *wfd)
fsd.frame_size = 0;
fsd.msg_idx_ctrl = 0;

/*
* RFC 6455 section 5.1: the server MUST close the connection on
* receiving a frame that is not masked. Without this check an
* unmasked frame is mis-parsed, since the first payload bytes are
* then consumed as the (absent) 4-byte masking key.
*/
if (!(fsd.mask & 0x80))
{
DEBUG("Client sent an unmasked frame!\n");
do_close(wfd, WS_CLSE_PROTERR);
wfd->error = 1;
break;
}

/*
* We should deny non-FIN control frames or that have
* more than 125 octets.
Expand Down Expand Up @@ -1786,6 +1805,19 @@ static void *ws_establishconnection(void *vclient)
if (do_handshake(&wfd) < 0)
goto closed;

/*
* Handshake done: drop the receive timeout that ws_accept() set, so an
* established session may stay idle on the read side indefinitely --
* long-running WebSocket connections are a first-class use case. The
* timeout only needs to bound the handshake against a slowloris client.
*/
if (timeout)
{
struct timeval zero = {0, 0};
setsockopt(client->client_sock, SOL_SOCKET, SO_RCVTIMEO,
(const char *)&zero, sizeof(zero));
}

/* Read next frame until client disconnects or an error occur. */
while (next_complete_frame(&wfd) >= 0)
{
Expand Down Expand Up @@ -1903,6 +1935,19 @@ static void *ws_accept(void *data)
*/
setsockopt(new_sock, SOL_SOCKET, SO_SNDTIMEO, (const char*)&time,
sizeof(struct timeval));

/*
* Bound the *handshake* read too. A client that connects and
* then sends nothing parks its handler thread indefinitely in
* do_handshake(); with the MAX_CLIENTS pool a few such idle
* sockets deny service to everyone else (a trivial slowloris).
* ws_establishconnection() clears this the moment the handshake
* completes, so an established long-running session is free to
* stay idle on the read side for as long as it likes. Opt-in via
* timeout_ms, so behaviour is unchanged when it is left at 0.
*/
setsockopt(new_sock, SOL_SOCKET, SO_RCVTIMEO, (const char*)&time,
sizeof(struct timeval));
}

/* Adds client socket to socks list. */
Expand Down