From 23572e043e32c6657005afbac3270f7be01e070a Mon Sep 17 00:00:00 2001 From: glmfe Date: Sun, 15 Jun 2025 18:19:16 -0300 Subject: [PATCH] fix(tcp_transport): off-by-one buffer corruption when WS header buffer full - Fix out of boundaries access - Improve test cases to cover this issue --- .../host_test/main/test_websocket_transport.cpp | 9 +++++++-- components/tcp_transport/transport_ws.c | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/components/tcp_transport/host_test/main/test_websocket_transport.cpp b/components/tcp_transport/host_test/main/test_websocket_transport.cpp index a13782ee97..4cddd83af5 100644 --- a/components/tcp_transport/host_test/main/test_websocket_transport.cpp +++ b/components/tcp_transport/host_test/main/test_websocket_transport.cpp @@ -220,7 +220,6 @@ TEST_CASE("WebSocket Transport Connection", "[success]") "Sec-WebSocket-Accept:\r\n" "\r\n"; REQUIRE(std::string(response_header_buffer.data()) == expected_header); - char buffer[WS_BUFFER_SIZE]; int read_len = 0; int partial_read; @@ -245,12 +244,18 @@ TEST_CASE("WebSocket Transport Connection", "[success]") esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0); mock_destroy_ExpectAnyArgsAndReturn(ESP_OK); + // Create a marker to check that the value after the end of the response header buffer is not overwritten + std::string expected_full_header = make_response(); + char marker = static_cast(~expected_full_header[ws_config.response_headers_len]); + response_header_buffer[ws_config.response_headers_len] = marker; + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); // Verify the response header was stored correctly. it must contain only ten bytes and be null terminated - std::string expected_header = "HTTP/1.1 1\0"; + std::string expected_header = "HTTP/1.1 \0"; REQUIRE(std::string(response_header_buffer.data()) == expected_header); + REQUIRE(response_header_buffer[ws_config.response_headers_len] == marker); } } diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 255c5cac5b..b814d75353 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -312,13 +312,13 @@ static int ws_connect(esp_transport_handle_t t, const char *host, int port, int } if(ws->response_header) { - if(ws->response_header_len < header_len) { + if(ws->response_header_len - 1 < header_len) { ESP_LOGW(TAG, "Received header length exceedes the allocated buffer size (need=%d, allocated=%d), truncating to allocated size", header_len, ws->response_header_len); header_len = ws->response_header_len; } // Copy response header to the static array strncpy(ws->response_header, ws->buffer, header_len); - ws->response_header[header_len] = '\0'; + ws->response_header[ws->response_header_len - 1] = '\0'; } char* delim_ptr = strstr(ws->buffer, delimiter);