From ef2298b91605728a1aec02b12f26fcf45af54716 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:28:25 -0400 Subject: [PATCH 1/9] fix: verify Sec-WebSocket-Accept against computed value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handshake was accepted whenever the response contained "HTTP/1.1 101" and a Sec-WebSocket-Accept header of any value; the accept digest computed from our nonce at construction was never compared (dead code). RFC 6455 §4.1 requires failing the connection unless Sec-WebSocket-Accept equals base64(SHA1(key + GUID)). Extract the header value (case-insensitive name, case-sensitive base64 value, trimmed) and compare it to the expected accept; fail the upgrade on mismatch or absence. --- src/WebSocketContext.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index 2147773..87e803c 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -639,10 +639,32 @@ void WebSocketContext::handleRead(bufferevent* bev) { //log_debug("RESP: %s", resp.c_str()); - if (resp.find("HTTP/1.1 101", 0) == std::string::npos || - !containsHeader(resp, "Sec-WebSocket-Accept:")) + // RFC 6455 §4.1: the client MUST fail the connection unless the response + // is 101 AND Sec-WebSocket-Accept equals base64(SHA1(key + GUID)). + // Accepting on mere header presence would let any 101-returning endpoint + // (a stray HTTP responder, a cache, an off-path injector) masquerade as a + // valid WebSocket peer. `accept` was computed from our nonce at construction. + auto headerValue = [&resp](const char* lowerName) -> std::string { + std::string lower = resp; + std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); + const size_t p = lower.find(lowerName); + if (p == std::string::npos) return std::string(); + const size_t vstart = p + std::char_traits::length(lowerName); + size_t lineEnd = resp.find("\r\n", vstart); + if (lineEnd == std::string::npos) lineEnd = resp.size(); + // Value from the original-case buffer (base64 is case-sensitive), trimmed. + const size_t a = resp.find_first_not_of(" \t", vstart); + if (a == std::string::npos || a >= lineEnd) return std::string(); + size_t b = lineEnd; + while (b > a && (resp[b - 1] == ' ' || resp[b - 1] == '\t' || resp[b - 1] == '\r')) --b; + return resp.substr(a, b - a); + }; + + const bool is101 = resp.find("HTTP/1.1 101", 0) != std::string::npos; + const std::string acceptValue = headerValue("sec-websocket-accept:"); + if (!is101 || acceptValue.empty() || acceptValue != accept) { - log_error("WebSocket upgrade failed"); + log_error("WebSocket upgrade failed (status/accept mismatch)"); connection_state.store(ConnectionState::FAILED, std::memory_order_release); sendError(ErrorCode::CONNECT_FAILED, "WebSocket upgrade failed"); evbuffer_drain(input, len); From ef32bb2a489958a24728973703a1c5ccd4d8b5f1 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:30:02 -0400 Subject: [PATCH 2/9] fix: bound permessage-deflate output (decompression bomb) rxInflate appended inflate output with no size limit, so a hostile server that negotiated permessage-deflate could send a small compressed frame that expands to gigabytes, exhausting memory. Cap the reassembled/inflated output at MAX_MESSAGE_SIZE (16 MiB) and abort inflation with a protocol error once it would be exceeded. The overflow check is written as `produced > MAX - old` to avoid any additive overflow. --- src/WebSocketReceiver.cpp | 5 +++++ src/WebSocketReceiver.h | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/WebSocketReceiver.cpp b/src/WebSocketReceiver.cpp index 7ca81bf..e4b0aaa 100644 --- a/src/WebSocketReceiver.cpp +++ b/src/WebSocketReceiver.cpp @@ -279,6 +279,11 @@ bool WebSocketReceiver::rxInflate(const uint8_t* in, size_t in_len, std::vector< if (produced) { const size_t old = out.size(); + if (produced > MAX_MESSAGE_SIZE - old) { + log_error("permessage-deflate output exceeds %zu bytes; aborting (possible decompression bomb)", + static_cast(MAX_MESSAGE_SIZE)); + return false; + } out.resize(old + produced); std::memcpy(out.data() + old, tmp, produced); } diff --git a/src/WebSocketReceiver.h b/src/WebSocketReceiver.h index 01457d8..196190c 100644 --- a/src/WebSocketReceiver.h +++ b/src/WebSocketReceiver.h @@ -79,4 +79,10 @@ class WebSocketReceiver { Utf8Validator utf8Validator; bool isValidUtf8(const char *str, size_t len); + + // Upper bound on a fully reassembled / inflated message. permessage-deflate + // lets a tiny frame expand without limit (a decompression bomb), so the + // inflate output is capped here. Audio-stream control/media messages are a + // few KB; 16 MiB is generous headroom while keeping memory bounded. + static constexpr size_t MAX_MESSAGE_SIZE = 16u * 1024u * 1024u; }; From 07dc1cbab0fdfa6ef4429dc85c37c86b4cec4005 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:30:42 -0400 Subject: [PATCH 3/9] fix: bound single-frame and reassembled message size The receiver imposed no maximum on a single frame's declared payload length (a peer-controlled 64-bit value) nor on the total size of a fragmented message accumulated across continuation frames, so a hostile server could drive unbounded buffering (memory exhaustion). Reject any data/continuation frame whose payload exceeds MAX_MESSAGE_SIZE and abort reassembly once the accumulated fragmented message would exceed it, both with close code 1009 (message too big). Control frames were already capped at 125 bytes. --- src/WebSocketReceiver.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/WebSocketReceiver.cpp b/src/WebSocketReceiver.cpp index e4b0aaa..a3c381f 100644 --- a/src/WebSocketReceiver.cpp +++ b/src/WebSocketReceiver.cpp @@ -364,6 +364,14 @@ void WebSocketReceiver::onData(evbuffer* buf) { return; } + // Bound a single data/continuation frame so a peer-declared 64-bit length + // cannot make us buffer (and reserve/copy) unbounded memory before the + // frame is even complete. Control frames are already capped at 125 above. + if ((opcode & 0x08) == 0 && payload_len > MAX_MESSAGE_SIZE) { + _sinks.onRxProtocolError(1009, "Frame payload too large"); + return; + } + const size_t need = header_len + static_cast(payload_len); if (data_len < need) break; // wait for full frame @@ -412,6 +420,13 @@ void WebSocketReceiver::handleContinuationFrame(const unsigned char* payload, si return; } + if (payload_len > MAX_MESSAGE_SIZE - fragmented_message.size()) { + log_error("reassembled message exceeds %zu bytes; aborting", + static_cast(MAX_MESSAGE_SIZE)); + _sinks.onRxProtocolError(1009, "Message too large"); + return; + } + fragmented_message.insert(fragmented_message.end(), payload, payload + payload_len); From 82edf7b8275ae8cdd876fbfcb4792379f3fac8cc Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:31:23 -0400 Subject: [PATCH 4/9] fix: seed frame-mask PRNG from a strong entropy source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-frame WebSocket masking key came from a splitmix PRNG seeded once from time(nullptr) XOR a stack address, which is predictable. RFC 6455 §5.3 requires masks derived from a strong source of entropy (the handshake nonce already uses std::random_device). Seed the PRNG from random_device; the per-frame splitmix step is retained so masking stays syscall-free on the hot path. --- src/WebSocketContext.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index 87e803c..582470a 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -1072,9 +1072,13 @@ void WebSocketContext::send(evbuffer* buf, const void* raw_data, size_t raw_len, thread_local uint32_t s = 0; if (s == 0) { - uint64_t t = static_cast(time(nullptr)); - uintptr_t a = reinterpret_cast(&s); - s = static_cast((t ^ (t >> 32) ^ a) | 1u); + // RFC 6455 §5.3: the masking key must be unpredictable. Seed the + // per-frame PRNG from a strong entropy source (as getWebSocketKey does + // for the handshake nonce) rather than time()+stack-address, which is + // guessable. The splitmix step below then produces per-frame masks + // without a syscall per frame. + std::random_device rd; + s = (static_cast(rd()) ^ static_cast(rd())) | 1u; } auto next_u32 = [&]() -> uint32_t { From 2d282c1a8798e643b2b4f0bfbd7c789c0390f9c5 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:32:08 -0400 Subject: [PATCH 5/9] fix: bound pre-upgrade handshake response size Before the WebSocket upgrade completes, every read copied the entire input buffer and rescanned it from the start for the CRLFCRLF header terminator, with no cap on the pre-upgrade byte count. A server that streams header bytes without ever terminating could grow the buffer and impose quadratic rescan cost. Fail the connection once the un-terminated handshake response exceeds 64 KiB. --- src/WebSocketContext.cpp | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index 582470a..164e7e6 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -619,13 +619,18 @@ void WebSocketContext::handleRead(bufferevent* bev) { if (!upgraded.load()) { + // Cap the pre-upgrade handshake response. Without this a server could + // stream header bytes forever (never sending the terminating CRLFCRLF), + // growing this buffer and re-scanning it from the start on every read. + static constexpr size_t MAX_HANDSHAKE_BYTES = 64u * 1024u; + const size_t len = evbuffer_get_length(input); if (len < 4) return; std::vector snap(len); evbuffer_copyout(input, snap.data(), len); const char* b = snap.data(); - + // Find end of headers: "\r\n\r\n" (length-bounded) size_t headerBytes = 0; for (size_t i = 0; i + 3 < len; ++i) { @@ -634,7 +639,17 @@ void WebSocketContext::handleRead(bufferevent* bev) { break; } } - if (headerBytes == 0) return; + if (headerBytes == 0) { + if (len > MAX_HANDSHAKE_BYTES) { + log_error("handshake response exceeded %zu bytes without header terminator", + static_cast(MAX_HANDSHAKE_BYTES)); + connection_state.store(ConnectionState::FAILED, std::memory_order_release); + sendError(ErrorCode::CONNECT_FAILED, "handshake response too large"); + evbuffer_drain(input, len); + requestLoopExit(); + } + return; // wait for more header bytes + } std::string resp(b, headerBytes); //log_debug("RESP: %s", resp.c_str()); From 666ea08065da15903ab8b5acd6e739aca8abd43f Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:32:48 -0400 Subject: [PATCH 6/9] fix: reject RSV1 on control and continuation frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When permessage-deflate was negotiated the receiver accepted the RSV1 (compression) bit on any frame. RFC 7692 §6 permits RSV1 only on the first frame of a message; it is illegal on control frames and on continuation frames. Reject those as a 1002 protocol error. --- src/WebSocketReceiver.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/WebSocketReceiver.cpp b/src/WebSocketReceiver.cpp index a3c381f..48e22bf 100644 --- a/src/WebSocketReceiver.cpp +++ b/src/WebSocketReceiver.cpp @@ -335,6 +335,14 @@ void WebSocketReceiver::onData(evbuffer* buf) { return; } + // RFC 7692 §6: the per-message-deflate RSV1 bit may only appear on the + // first frame of a message. It is illegal on control frames and on + // continuation frames even when compression is negotiated. + if (rsv1 && ((opcode & 0x08) != 0 || opcode == 0x00)) { + _sinks.onRxProtocolError(1002, "RSV1 set on control or continuation frame"); + return; + } + if ((opcode & 0x08) != 0 && !fin) { _sinks.onRxProtocolError(1002, "Control frame fragmented"); return; From d113768ab242cba734a09102a58f67f098aead7f Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:33:32 -0400 Subject: [PATCH 7/9] fix: verify IP-literal TLS endpoints against iPAddress SANs For a wss:// endpoint given as an IP literal, peer verification used X509_VERIFY_PARAM_set1_host, which matches dNSName SANs, so a certificate with the correct iPAddress SAN could be rejected (or the IP not validated as intended). Use X509_VERIFY_PARAM_set1_ip_asc for IP-literal hosts and keep set1_host for hostnames. --- src/WebSocketContext.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index 164e7e6..a6104bd 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -182,9 +182,16 @@ void WebSocketContext::run() { if (!_cfg.tls.disableHostnameValidation) { X509_VERIFY_PARAM* param = SSL_get0_param(ssl); if (param) { - int ret = X509_VERIFY_PARAM_set1_host(param, _cfg.host.c_str(), 0); // No port matching + // For an IP-literal endpoint the peer identity must be checked + // against the certificate's iPAddress SANs (set1_ip), not its + // dNSName SANs (set1_host); using set1_host on an IP would mis- + // validate. Hostnames use set1_host. + int ret = _cfg.is_ip_address + ? X509_VERIFY_PARAM_set1_ip_asc(param, _cfg.host.c_str()) + : X509_VERIFY_PARAM_set1_host(param, _cfg.host.c_str(), 0); // No port matching if (ret != 1) { - log_error("Failed to set hostname for verification"); + log_error("Failed to set %s for verification", + _cfg.is_ip_address ? "IP address" : "hostname"); sendError(ErrorCode::TLS_INIT_FAILED, "Failed hostname verification setup"); SSL_free(ssl); _tls.reset(); From 7d8d07d9a1bc769d85fc1db7547e66f38afc5e20 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:34:27 -0400 Subject: [PATCH 8/9] fix: strip CR/LF from configured URI/host/headers in handshake The handshake request interpolated the configured URI, host, and custom header key/values directly into CRLF-terminated request lines. A CR or LF embedded in any of those (via local configuration) would inject arbitrary handshake headers or smuggle a second request. Strip CR/LF/NUL from these values before writing the request line. --- src/WebSocketContext.cpp | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index a6104bd..a21177f 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -839,28 +839,45 @@ void WebSocketContext::flushSendQueue() { } } +// Strip CR/LF/NUL from a value interpolated into a request line, so a +// configured URI/host/header cannot inject additional handshake headers or +// smuggle a second request (header/request splitting). +static std::string stripCRLF(const std::string& s) { + std::string out; + out.reserve(s.size()); + for (char c : s) { + if (c != '\r' && c != '\n' && c != '\0') out.push_back(c); + } + return out; +} + void WebSocketContext::sendHandshakeRequest() { if (!_bev) return; log_debug("Sending WebSocket handshake request"); auto out = bufferevent_get_output(_bev); - evbuffer_add_printf(out, "GET %s HTTP/1.1\r\n", _cfg.uri.c_str()); - evbuffer_add_printf(out, "Host:%s:%d\r\n", _cfg.host.c_str(), _cfg.port); + const std::string uri = stripCRLF(_cfg.uri); + const std::string host = stripCRLF(_cfg.host); + + evbuffer_add_printf(out, "GET %s HTTP/1.1\r\n", uri.c_str()); + evbuffer_add_printf(out, "Host:%s:%d\r\n", host.c_str(), _cfg.port); evbuffer_add_printf(out, "Upgrade:websocket\r\n"); evbuffer_add_printf(out, "Connection:upgrade\r\n"); evbuffer_add_printf(out, "Sec-WebSocket-Key:%s\r\n", key.c_str()); evbuffer_add_printf(out, "Sec-WebSocket-Version:13\r\n"); - + if (_cfg.compression_requested) { evbuffer_add_printf(out, "Sec-WebSocket-Extensions:permessage-deflate; client_no_context_takeover; server_no_context_takeover; client_max_window_bits=9\r\n"); } - evbuffer_add_printf(out, "Origin:http://%s:%d\r\n", _cfg.host.c_str(), _cfg.port); - + evbuffer_add_printf(out, "Origin:http://%s:%d\r\n", host.c_str(), _cfg.port); + if (!_cfg.headers.headers.empty()) { for (const auto& header : _cfg.headers.headers) { - evbuffer_add_printf(out, "%s:%s\r\n", header.first.c_str(), header.second.c_str()); + evbuffer_add_printf(out, "%s:%s\r\n", + stripCRLF(header.first).c_str(), + stripCRLF(header.second).c_str()); } } From 33aa14479326c7d3f6b3a20c39bcc3d0fb906097 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:36:57 -0400 Subject: [PATCH 9/9] fix: detect half-open peer via ping/pong liveness timeout Heartbeat pings were fire-and-forget: pongs were never tracked and the declared ErrorCode::PING_TIMEOUT was unused, so a peer that went silent while its TCP connection stayed up was never detected. Track pings sent since the last pong (event-thread-only state) and, once MAX_MISSED_PONGS (2) intervals pass with no pong, declare the connection dead using the same teardown path as other fatal errors. Gated on the upgrade and on ping_interval > 0, so behavior is unchanged when the heartbeat is off. --- src/WebSocketContext.cpp | 16 ++++++++++++++++ src/WebSocketContext.h | 7 +++++++ 2 files changed, 23 insertions(+) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index a21177f..a44cdd7 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -353,7 +353,21 @@ void WebSocketContext::timeoutCallback(evutil_socket_t /*fd*/, short /*event*/, void WebSocketContext::pingCallback(evutil_socket_t /*fd*/, short /*event*/, void *arg) { auto* self = static_cast(arg); + // Heartbeat only runs after the upgrade; sendPing() is a no-op before then. + if (!self->upgraded.load(std::memory_order_acquire)) return; + + // If earlier pings have gone unanswered for MAX_MISSED_PONGS intervals the + // peer is half-open (TCP up, no application response). Declare the + // connection dead, mirroring the fatal-error teardown in handleEvent. + if (self->pings_outstanding >= MAX_MISSED_PONGS) { + log_error("ping timeout: %d unanswered ping(s)", self->pings_outstanding); + self->sendError(ErrorCode::PING_TIMEOUT, "Ping timeout (no pong)"); + self->connection_state.store(ConnectionState::DISCONNECTING, std::memory_order_release); + self->requestLoopExit(); + return; + } self->sendPing(); + self->pings_outstanding++; } void WebSocketContext::wakeupCallback(evutil_socket_t, short, void* arg) { @@ -1236,6 +1250,8 @@ bool WebSocketContext::rxCompressionEnabled() const { void WebSocketContext::onRxPong(std::vector&& payload) { log_debug("Received pong frame (%zu bytes)", payload.size()); (void)payload; + // Peer is alive; reset the heartbeat liveness counter. + pings_outstanding = 0; } void WebSocketContext::onRxPing(std::vector&& payload) { diff --git a/src/WebSocketContext.h b/src/WebSocketContext.h index 1a97dae..0a79664 100644 --- a/src/WebSocketContext.h +++ b/src/WebSocketContext.h @@ -219,6 +219,13 @@ class WebSocketContext: public std::enable_shared_from_this, struct event *ping_event = nullptr; struct event *wakeup_event = nullptr; + // Heartbeat liveness: number of pings sent since the last pong. Touched only + // on the event thread (pingCallback / onRxPong). The connection is declared + // dead once this reaches MAX_MISSED_PONGS, detecting a half-open peer that + // stopped responding while TCP stayed up. + int pings_outstanding = 0; + static constexpr int MAX_MISSED_PONGS = 2; + // Sender struct event *send_event = nullptr; std::atomic_bool send_flush_pending{false};