From 9c80011c3683095c71f0cf7c1da8d210201141ec Mon Sep 17 00:00:00 2001 From: Suren Gabrielyan Date: Fri, 12 Dec 2025 14:01:18 +0400 Subject: [PATCH 1/8] fix(transport_ws): fix ws buffer overflow check and missing delimiter handling --- components/tcp_transport/transport_ws.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 7a554771ca7..d32cc167f6a 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -304,12 +304,16 @@ static int ws_connect(esp_transport_handle_t t, const char *host, int port, int ESP_LOGD(TAG, "Read header chunk %d, current header size: %d", len, header_len); } while (NULL == strstr(ws->buffer, delimiter) && header_len < WS_BUFFER_SIZE - 1); - if (header_len >= WS_BUFFER_SIZE - 1) { - ESP_LOGE(TAG, "Header size exceeded buffer size"); + if (header_len > WS_BUFFER_SIZE - 1) { + ESP_LOGE(TAG, "Header size exceeded buffer size (need=%d, max=%d)", header_len + 1, WS_BUFFER_SIZE); return -1; } char* delim_ptr = strstr(ws->buffer, delimiter); + if (!delim_ptr) { + ESP_LOGE(TAG, "Header size exceeded buffer size or delimiter not found"); + return -1; + } ws->http_status_code = get_http_status_code(ws->buffer); if (ws->http_status_code == -1) { From 524893585551245f1a35fcb65478d95b0b119678 Mon Sep 17 00:00:00 2001 From: surengab Date: Fri, 23 Jan 2026 14:39:02 +0400 Subject: [PATCH 2/8] fix(transport_ws): optimize ws polling and fix read error logging --- components/tcp_transport/transport_ws.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index d32cc167f6a..01b132720f4 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -482,7 +482,9 @@ static int ws_read_payload(esp_transport_handle_t t, char *buffer, int len, int // Receive and process payload if (bytes_to_read != 0 && (rlen = esp_transport_read_internal(ws, buffer, bytes_to_read, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data(%d)", rlen); + if (rlen < 0) { + ESP_LOGE(TAG, "Error read data(%d)", rlen); + } return rlen; } ws->frame_state.bytes_remaining -= rlen; @@ -531,10 +533,12 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t char ws_header[MAX_WEBSOCKET_HEADER_SIZE]; char *data_ptr = ws_header, mask; int rlen; - int poll_read; ws->frame_state.header_received = false; - if ((poll_read = esp_transport_poll_read(ws->parent, timeout_ms)) <= 0) { - return poll_read; + if (ws->buffer_len == 0) { + int poll_read = esp_transport_poll_read(ws->parent, timeout_ms); + if (poll_read <= 0) { + return poll_read; + } } // Receive and process header first (based on header size) @@ -673,8 +677,10 @@ static int ws_read(esp_transport_handle_t t, char *buffer, int len, int timeout_ if (ws->frame_state.payload_len) { if ( (rlen = ws_read_payload(t, buffer, len, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error reading payload data(%d)", rlen); - ws->frame_state.bytes_remaining = 0; + if (rlen < 0) { + ESP_LOGE(TAG, "Error reading payload data(%d)", rlen); + ws->frame_state.bytes_remaining = 0; + } return rlen; } } @@ -686,6 +692,10 @@ static int ws_read(esp_transport_handle_t t, char *buffer, int len, int timeout_ static int ws_poll_read(esp_transport_handle_t t, int timeout_ms) { transport_ws_t *ws = esp_transport_get_context_data(t); + if (ws->buffer_len > 0) { + ESP_LOGD(TAG, "ws_poll_read: buffered data available (%zu bytes)", ws->buffer_len); + return 1; + } return esp_transport_poll_read(ws->parent, timeout_ms); } From 4b12a29e9b3c32f2e7f4170f3885c760d5506639 Mon Sep 17 00:00:00 2001 From: surengab Date: Mon, 2 Feb 2026 14:59:56 +0400 Subject: [PATCH 3/8] test(transport_ws): add tests for polling logic and handshake edge cases --- .../main/test_websocket_transport.cpp | 87 ++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) 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 15599e626d0..83086ce8bb4 100644 --- a/components/tcp_transport/host_test/main/test_websocket_transport.cpp +++ b/components/tcp_transport/host_test/main/test_websocket_transport.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -208,6 +208,65 @@ TEST_CASE("WebSocket Transport Connection", "[success]") std::string response(buffer, read_len); REQUIRE(response == "Test"); } + + SECTION("Happy flow with smaller response header") { + // Set the response header length to a size that's smaller than the full response + // but still large enough to find the header delimiter + ws_config.response_headers_len = 130; // Large enough for the header but smaller than full response + REQUIRE(esp_transport_ws_set_config(websocket_transport.get(), &ws_config) == ESP_OK); + + // Set the callback function for mock_read + mock_read_Stub(mock_valid_read_callback); + mock_poll_read_Stub(mock_poll_read_callback); + + // Create a marker to check that the value after the end of the response header buffer is not overwritten + std::string expected_full_response = make_response(); + char marker = 0x42; // Use a distinctive marker value + 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 and truncated at the header boundary + std::string expected_header = "HTTP/1.1 101 Switching Protocols\r\n" + "Upgrade: websocket\r\n" + "Connection: Upgrade\r\n" + "Sec-WebSocket-Accept: HSmrc0sMlYUkAGmm5OPpG2HaGWk=\r\n" + "\r\n"; + + REQUIRE(std::string(response_header_buffer.data()) == expected_header); + // Verify the marker after the buffer wasn't overwritten + REQUIRE(response_header_buffer[ws_config.response_headers_len] == marker); + } + + SECTION("Poll read with buffered data") { + // Set the callback function for mock_read + mock_read_Stub(mock_valid_read_callback); + + static int parent_poll_calls = 0; + parent_poll_calls = 0; + + // Verify poll_read is not called when buffer has data + mock_poll_read_Stub([](esp_transport_handle_t t, int timeout_ms, int num_call){ + parent_poll_calls++; + return 0; + }); + + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); + + // buffer should contain "Test" (4 bytes) + // ws_poll_read should return 1 because buffer is not empty + REQUIRE(esp_transport_poll_read(websocket_transport.get(), timeout) == 1); + REQUIRE(parent_poll_calls == 0); + + // Read the data to empty the buffer + char buffer[10]; + int read_len = esp_transport_read(websocket_transport.get(), buffer, sizeof(buffer), timeout); + REQUIRE(read_len == 4); + + // Now buffer is empty, ws_poll_read should call parent poll + esp_transport_poll_read(websocket_transport.get(), timeout); + REQUIRE(parent_poll_calls == 1); + } } TEST_CASE("WebSocket Transport Connection", "[failure]") @@ -314,4 +373,30 @@ TEST_CASE("WebSocket Transport Connection", "[failure]") // Assert the expected HTTP status code REQUIRE((esp_transport_ws_get_upgrade_request_status(websocket_transport.get())) == 301); } + + SECTION("ws connect fails (buffer full, no delimiter)") { + // Mock read to fill buffer with non-delimiter data + mock_read_Stub([](esp_transport_handle_t h, char *buf, int len, int tout, int n) { + if (len > 0) { + memset(buf, 'A', len); + } + return len; + }); + mock_poll_read_Stub(mock_poll_read_callback); + + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == -1); + } + + SECTION("ws connect succeeds (response header buffer too small for handshake verification)") { + // Set a very small response header buffer + ws_config.response_headers_len = 50; + REQUIRE(esp_transport_ws_set_config(websocket_transport.get(), &ws_config) == ESP_OK); + + // Set the callback function for mock_read + mock_read_Stub(mock_valid_read_callback); + mock_poll_read_Stub(mock_poll_read_callback); + + // Connect should now succeed even with small user buffer + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); + } } From 614c50b86e39f1a549e2824f6da8bf7898980f08 Mon Sep 17 00:00:00 2001 From: surengab Date: Mon, 2 Feb 2026 15:44:40 +0400 Subject: [PATCH 4/8] fix(transport_ws): fix handshake failure with small response buffer From 74afee2afc4ce4f5c3192187ab0b0b0bc1a01f3c Mon Sep 17 00:00:00 2001 From: surengab Date: Wed, 18 Feb 2026 15:16:29 +0400 Subject: [PATCH 5/8] fix(transport_ws): validate RSV bits per RFC 6455(reject frames with non-zero RSV) --- components/tcp_transport/transport_ws.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 01b132720f4..2949833678c 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -551,11 +551,18 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t ws->frame_state.header_received = true; ws->frame_state.fin = (*data_ptr & 0x80) != 0; ws->frame_state.opcode = (*data_ptr & 0x0F); + uint8_t rsv = (*data_ptr & 0x70); data_ptr ++; mask = ((*data_ptr >> 7) & 0x01); payload_len = (*data_ptr & 0x7F); data_ptr++; - ESP_LOGD(TAG, "Opcode: %d, mask: %d, len: %d", ws->frame_state.opcode, mask, payload_len); + ESP_LOGD(TAG, "Opcode: %d, mask: %d, len: %d, rsv: 0x%02X", ws->frame_state.opcode, mask, payload_len, rsv); + + // RFC 6455 Section 5.2: RSV bits MUST be 0 unless an extension is negotiated + if (rsv != 0) { + ESP_LOGE(TAG, "Non-zero RSV bits detected (rsv=0x%02X) - protocol violation, no extensions negotiated", rsv); + return -1; + } if (payload_len == 126) { // headerLen += 2; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { From a0c18b68de520bd86feaed4e4076afbfe4f0b48d Mon Sep 17 00:00:00 2001 From: surengab Date: Wed, 18 Feb 2026 15:24:44 +0400 Subject: [PATCH 6/8] fix(transport_ws): reject reserved opcodes per RFC 6455 --- components/tcp_transport/transport_ws.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 2949833678c..6e6172fbaeb 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -563,6 +563,14 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t ESP_LOGE(TAG, "Non-zero RSV bits detected (rsv=0x%02X) - protocol violation, no extensions negotiated", rsv); return -1; } + + // RFC 6455 Section 5.2: Validate opcode (only 0x0-0x2 for data, 0x8-0xA for control are defined) + if ((ws->frame_state.opcode >= 0x3 && ws->frame_state.opcode <= 0x7) || + (ws->frame_state.opcode >= 0xB && ws->frame_state.opcode <= 0xF)) { + ESP_LOGE(TAG, "Reserved opcode detected (opcode=0x%02X) - protocol violation", ws->frame_state.opcode); + return -1; + } + if (payload_len == 126) { // headerLen += 2; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { From 465a6c372eea0a7ec5ab47590312573a90305deb Mon Sep 17 00:00:00 2001 From: surengab Date: Wed, 18 Feb 2026 15:26:55 +0400 Subject: [PATCH 7/8] fix(transport_ws): add RFC 6455 validation for fragmented control frames --- components/tcp_transport/transport_ws.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 6e6172fbaeb..473dadb4eeb 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -571,6 +571,13 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t return -1; } + // RFC 6455 Section 5.5: Control frames MUST NOT be fragmented + if ((ws->frame_state.opcode & WS_OPCODE_CONTROL_FRAME) && !ws->frame_state.fin) { + ESP_LOGE(TAG, "Fragmented control frame detected (opcode=0x%02X, fin=%d) - protocol violation", + ws->frame_state.opcode, ws->frame_state.fin); + return -1; + } + if (payload_len == 126) { // headerLen += 2; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { From 4bd1b3cf8421a80daaf3f929145b2051295defde Mon Sep 17 00:00:00 2001 From: surengab Date: Wed, 18 Feb 2026 15:28:36 +0400 Subject: [PATCH 8/8] fix(transport_ws): add RFC 6455 validation for control frame payload size --- components/tcp_transport/transport_ws.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 473dadb4eeb..2a991e92455 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -600,7 +600,12 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t payload_len = (uint8_t)data_ptr[4] << 24 | (uint8_t)data_ptr[5] << 16 | (uint8_t)data_ptr[6] << 8 | data_ptr[7]; } } - + // RFC 6455 Section 5.5: Control frames MUST have payload length of 125 bytes or less + if ((ws->frame_state.opcode & WS_OPCODE_CONTROL_FRAME) && payload_len > 125) { + ESP_LOGE(TAG, "Control frame with excessive payload detected (opcode=0x%02X, payload_len=%d) - protocol violation", + ws->frame_state.opcode, payload_len); + return -1; + } if (mask) { // Read and store mask if (payload_len != 0 && (rlen = esp_transport_read_exact_size(ws, buffer, mask_len, timeout_ms)) <= 0) {