From ef2298b91605728a1aec02b12f26fcf45af54716 Mon Sep 17 00:00:00 2001 From: Michael Mavroudis Date: Thu, 2 Jul 2026 08:28:25 -0400 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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 08/10] 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 09/10] 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}; From efbab3cb4e46fd3b46738fb9cb243ea08502e919 Mon Sep 17 00:00:00 2001 From: "Milan M." Date: Thu, 10 Sep 2026 20:16:35 +0200 Subject: [PATCH 10/10] Harden PR #7 security fixes --- src/WebSocketContext.cpp | 254 ++++++++++++++++++++++---------------- src/WebSocketContext.h | 5 +- src/WebSocketReceiver.cpp | 16 +-- src/WebSocketReceiver.h | 4 - 4 files changed, 159 insertions(+), 120 deletions(-) diff --git a/src/WebSocketContext.cpp b/src/WebSocketContext.cpp index a44cdd7..b428711 100644 --- a/src/WebSocketContext.cpp +++ b/src/WebSocketContext.cpp @@ -182,17 +182,15 @@ void WebSocketContext::run() { if (!_cfg.tls.disableHostnameValidation) { X509_VERIFY_PARAM* param = SSL_get0_param(ssl); if (param) { - // 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. + // Verify IP addresses against IP SANs, hostnames against DNS SANs. 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 + : X509_VERIFY_PARAM_set1_host(param, _cfg.host.c_str(), 0); if (ret != 1) { log_error("Failed to set %s for verification", _cfg.is_ip_address ? "IP address" : "hostname"); - sendError(ErrorCode::TLS_INIT_FAILED, "Failed hostname verification setup"); + sendError(ErrorCode::TLS_INIT_FAILED, + _cfg.is_ip_address ? "Failed IP address verification setup" : "Failed hostname verification setup"); SSL_free(ssl); _tls.reset(); return; @@ -353,12 +351,10 @@ 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. + // Heartbeat only runs after the WebSocket upgrade. 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. + // Disconnect after too many unanswered pings. 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)"); @@ -640,27 +636,14 @@ 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(); + const evbuffer_ptr headerEnd = evbuffer_search(input, "\r\n\r\n", 4, nullptr); - // Find end of headers: "\r\n\r\n" (length-bounded) - size_t headerBytes = 0; - for (size_t i = 0; i + 3 < len; ++i) { - if (b[i] == '\r' && b[i+1] == '\n' && b[i+2] == '\r' && b[i+3] == '\n') { - headerBytes = i + 4; - break; - } - } - if (headerBytes == 0) { + if (headerEnd.pos < 0) { if (len > MAX_HANDSHAKE_BYTES) { log_error("handshake response exceeded %zu bytes without header terminator", static_cast(MAX_HANDSHAKE_BYTES)); @@ -671,33 +654,101 @@ void WebSocketContext::handleRead(bufferevent* bev) { } return; // wait for more header bytes } + + const size_t headerBytes = static_cast(headerEnd.pos) + 4; + + if (headerBytes > MAX_HANDSHAKE_BYTES) { + log_error("handshake response exceeded %zu bytes", + 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; + } + + // Copy only the HTTP handshake headers, not any WebSocket data + // that may already follow in the same input buffer. + std::vector snap(headerBytes); + evbuffer_copyout(input, snap.data(), headerBytes); + + const char* b = snap.data(); + std::string resp(b, headerBytes); //log_debug("RESP: %s", resp.c_str()); - // 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); + // RFC 6455: validate the HTTP 101 status and Sec-WebSocket-Accept value. + auto headerValue = [&resp](const std::string& expectedName) -> std::string { + size_t pos = 0; + + while (pos < resp.size()) { + size_t lineEnd = resp.find("\r\n", pos); + if (lineEnd == std::string::npos) { + lineEnd = resp.size(); + } + + if (lineEnd == pos) { + break; + } + + const size_t colon = resp.find(':', pos); + + if (colon != std::string::npos && colon < lineEnd) { + std::string headerName = resp.substr(pos, colon - pos); + + std::transform( + headerName.begin(), + headerName.end(), + headerName.begin(), + [](unsigned char c) { + return static_cast(std::tolower(c)); + }); + + if (headerName == expectedName) { + size_t valueStart = colon + 1; + + while (valueStart < lineEnd && + (resp[valueStart] == ' ' || resp[valueStart] == '\t')) { + ++valueStart; + } + + size_t valueEnd = lineEnd; + + while (valueEnd > valueStart && + (resp[valueEnd - 1] == ' ' || + resp[valueEnd - 1] == '\t')) { + --valueEnd; + } + + return resp.substr(valueStart, valueEnd - valueStart); + } + } + + if (lineEnd == resp.size()) { + break; + } + + pos = lineEnd + 2; + } + + return {}; }; - const bool is101 = resp.find("HTTP/1.1 101", 0) != std::string::npos; - const std::string acceptValue = headerValue("sec-websocket-accept:"); + const size_t statusEnd = resp.find("\r\n"); + + const bool is101 = statusEnd != std::string::npos && + resp.size() >= 12 && + resp.compare(0, 12, "HTTP/1.1 101") == 0 && + (statusEnd == 12 || resp[12] == ' ' || resp[12] == '\t'); + + const std::string acceptValue = headerValue("sec-websocket-accept"); + if (!is101 || acceptValue.empty() || acceptValue != accept) { log_error("WebSocket upgrade failed (status/accept mismatch)"); @@ -711,14 +762,9 @@ void WebSocketContext::handleRead(bufferevent* bev) { bool negotiated = false; if (_cfg.compression_requested) { - std::string lowerResp = resp; - std::transform(lowerResp.begin(), lowerResp.end(), lowerResp.begin(), ::tolower); - const std::string key = "sec-websocket-extensions:"; - size_t extHeaderPos = lowerResp.find(key); - if (extHeaderPos != std::string::npos) { - size_t lineEnd = resp.find("\r\n", extHeaderPos); - if (lineEnd == std::string::npos) lineEnd = resp.size(); - std::string extLine = resp.substr(extHeaderPos, lineEnd - extHeaderPos); + const std::string extLine = headerValue("sec-websocket-extensions"); + + if (!extLine.empty()) { if (containsHeader(extLine, "permessage-deflate")) { negotiated = true; @@ -853,50 +899,70 @@ 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; +static bool hasInvalidHandshakeChars(const std::string& s) { + return s.find('\r') != std::string::npos || + s.find('\n') != std::string::npos || + s.find('\0') != std::string::npos; } void WebSocketContext::sendHandshakeRequest() { if (!_bev) return; + log_debug("Sending WebSocket handshake request"); - auto out = bufferevent_get_output(_bev); + if (hasInvalidHandshakeChars(_cfg.uri) || + hasInvalidHandshakeChars(_cfg.host)) { - const std::string uri = stripCRLF(_cfg.uri); - const std::string host = stripCRLF(_cfg.host); + log_error("Invalid CR/LF/NUL character in WebSocket URI or host"); + sendError(ErrorCode::CONNECT_FAILED, + "Invalid WebSocket handshake configuration"); + requestLoopExit(); + return; + } + + for (const auto& header : _cfg.headers.headers) { + if (hasInvalidHandshakeChars(header.first) || + hasInvalidHandshakeChars(header.second)) { + + log_error("Invalid CR/LF/NUL character in WebSocket header"); + sendError(ErrorCode::CONNECT_FAILED, + "Invalid WebSocket handshake header"); + requestLoopExit(); + return; + } + } + + auto out = bufferevent_get_output(_bev); - 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, "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); 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, + "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", host.c_str(), _cfg.port); + evbuffer_add_printf(out, + "Origin:http://%s:%d\r\n", + _cfg.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", - stripCRLF(header.first).c_str(), - stripCRLF(header.second).c_str()); - } + 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, "\r\n"); - } void WebSocketContext::sendError(int error_code, const std::string& error_message) { @@ -1123,39 +1189,20 @@ void WebSocketContext::send(evbuffer* buf, const void* raw_data, size_t raw_len, // ---- Fast masking (single evbuffer_add) ---- uint8_t mask_key[4]; - thread_local uint32_t s = 0; - if (s == 0) { - // 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 { - s += 0x9E3779B9u; - uint32_t z = s; - z ^= z >> 16; - z *= 0x85EBCA6Bu; - z ^= z >> 13; - z *= 0xC2B2AE35u; - z ^= z >> 16; - return z; - }; - - uint32_t mask32 = next_u32(); - std::memcpy(mask_key, &mask32, 4); + thread_local std::random_device rd; + + const uint32_t mask32 = static_cast(rd()); + std::memcpy(mask_key, &mask32, sizeof(mask_key)); // Write mask key - evbuffer_add(out, mask_key, 4); + evbuffer_add(out, mask_key, sizeof(mask_key)); // Mask payload into one contiguous buffer, then add once static thread_local std::vector masked; masked.resize(payload_len); const uint8_t* src = payload_ptr; + for (size_t i = 0; i < payload_len; ++i) { masked[i] = src[i] ^ mask_key[i & 3]; } @@ -1250,7 +1297,6 @@ 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; } diff --git a/src/WebSocketContext.h b/src/WebSocketContext.h index 0a79664..ce1963d 100644 --- a/src/WebSocketContext.h +++ b/src/WebSocketContext.h @@ -219,10 +219,7 @@ 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. + // Ping liveness int pings_outstanding = 0; static constexpr int MAX_MISSED_PONGS = 2; diff --git a/src/WebSocketReceiver.cpp b/src/WebSocketReceiver.cpp index 48e22bf..cd558c4 100644 --- a/src/WebSocketReceiver.cpp +++ b/src/WebSocketReceiver.cpp @@ -279,7 +279,7 @@ 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) { + if (old > MAX_MESSAGE_SIZE || produced > MAX_MESSAGE_SIZE - old) { log_error("permessage-deflate output exceeds %zu bytes; aborting (possible decompression bomb)", static_cast(MAX_MESSAGE_SIZE)); return false; @@ -335,9 +335,7 @@ 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. + // RFC 7692: RSV1 is invalid on control and continuation frames. if (rsv1 && ((opcode & 0x08) != 0 || opcode == 0x00)) { _sinks.onRxProtocolError(1002, "RSV1 set on control or continuation frame"); return; @@ -372,9 +370,6 @@ 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; @@ -428,9 +423,14 @@ void WebSocketReceiver::handleContinuationFrame(const unsigned char* payload, si return; } - if (payload_len > MAX_MESSAGE_SIZE - fragmented_message.size()) { + const size_t current_size = fragmented_message.size(); + + if (current_size > MAX_MESSAGE_SIZE || + payload_len > static_cast(MAX_MESSAGE_SIZE - current_size)) + { log_error("reassembled message exceeds %zu bytes; aborting", static_cast(MAX_MESSAGE_SIZE)); + _sinks.onRxProtocolError(1009, "Message too large"); return; } diff --git a/src/WebSocketReceiver.h b/src/WebSocketReceiver.h index 196190c..e9389c2 100644 --- a/src/WebSocketReceiver.h +++ b/src/WebSocketReceiver.h @@ -80,9 +80,5 @@ 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; };