From fb38b3eb669859fecd72f99327fcff1d76a2c894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 31 Aug 2026 01:24:56 +0200 Subject: [PATCH] Net: three smaller fixes from reviewing the server code RequestHeader::GetParamValue indexed parts[1] without checking the size. A query parameter with no '=' at all ("?foo") makes SplitString return a single element, so both the DEBUG_LOG and the assignment read off the end of the vector. Nothing calls GetParamValue today, so this is latent rather than live, but it's driven straight off the request line. The 64-bit frame length was assembled with header[n] << 24 on uint8_t values, which promote to int - a byte >= 0x80 in the top position shifts into the sign bit and then sign-extends when widened to uint64_t. The resulting size was always rejected, just by the wrong check and via signed overflow to get there. Cast first. OutputSink::Block() had the same shape as the InputSink one this branch already fixed: a broken socket is reported ready immediately and forever, so waiting on it is a spin. Bail if the sink already knows it's broken. --- Common/Net/HTTPHeaders.cpp | 5 +++++ Common/Net/Sinks.cpp | 4 ++++ Common/Net/WebsocketServer.cpp | 7 ++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Common/Net/HTTPHeaders.cpp b/Common/Net/HTTPHeaders.cpp index 410ed63060..afa04b6a88 100644 --- a/Common/Net/HTTPHeaders.cpp +++ b/Common/Net/HTTPHeaders.cpp @@ -31,6 +31,11 @@ bool RequestHeader::GetParamValue(const char *param_name, std::string *value) co for (size_t i = 0; i < v.size(); i++) { std::vector parts; SplitString(v[i], '=', parts); + if (parts.size() < 2) { + // A parameter with no '=' at all, like "?foo". SplitString hands back a single element + // for that, so parts[1] would be reading off the end of the vector. + continue; + } DEBUG_LOG(Log::HTTP, "Param: %.*s Value: %.*s", (int)parts[0].size(), parts[0].data(), (int)parts[1].size(), parts[1].data()); if (parts[0] == param_name) { *value = parts[1]; diff --git a/Common/Net/Sinks.cpp b/Common/Net/Sinks.cpp index 2325fb2b19..194c605a3f 100644 --- a/Common/Net/Sinks.cpp +++ b/Common/Net/Sinks.cpp @@ -358,6 +358,10 @@ bool OutputSink::Printf(const char *fmt, ...) { } bool OutputSink::Block() { + if (hasError_) { + // A broken socket reports as ready forever, so waiting on it would just spin. + return false; + } if (!fd_util::WaitUntilReady((int)fd_, 5.0, true)) { return false; } diff --git a/Common/Net/WebsocketServer.cpp b/Common/Net/WebsocketServer.cpp index 5a3f31a25d..d017711c84 100644 --- a/Common/Net/WebsocketServer.cpp +++ b/Common/Net/WebsocketServer.cpp @@ -400,9 +400,10 @@ bool WebSocketServer::ReadFrame() { return false; mask = &header[10]; - // Read from big endian. - uint64_t high = (header[2] << 24) | (header[3] << 16) | (header[4] << 8) | (header[5] << 0); - uint64_t low = (header[6] << 24) | (header[7] << 16) | (header[8] << 8) | (header[9] << 0); + // Read from big endian. Cast first: these promote to int, so a byte >= 0x80 in the top + // position would shift into the sign bit and then sign-extend into the u64. + uint64_t high = ((uint64_t)header[2] << 24) | ((uint64_t)header[3] << 16) | ((uint64_t)header[4] << 8) | (uint64_t)header[5]; + uint64_t low = ((uint64_t)header[6] << 24) | ((uint64_t)header[7] << 16) | ((uint64_t)header[8] << 8) | (uint64_t)header[9]; sz = (high << 32) | low; if ((sz & 0x8000000000000000ULL) != 0) {