espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-08-05 05:34:32 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(tcp_transport): off-by-one buffer corruption when WS header buffer full

Browse Source 
- Fix out of boundaries access
 - Improve test cases to cover this issue
This commit is contained in:
glmfe
2025-06-15 18:19:16 -03:00
parent a319aa902c
commit 23572e043e
2 changed files with 9 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
components/tcp_transport/host_test/main/test_websocket_transport.cpp
View File

@@ -220,7 +220,6 @@ TEST_CASE("WebSocket Transport Connection", "[success]")
"Sec-WebSocket-Accept:\r\n" "Sec-WebSocket-Accept:\r\n"
"\r\n"; "\r\n";
REQUIRE(std::string(response_header_buffer.data()) == expected_header); REQUIRE(std::string(response_header_buffer.data()) == expected_header);
char buffer[WS_BUFFER_SIZE]; char buffer[WS_BUFFER_SIZE];
int read_len = 0; int read_len = 0;
int partial_read; int partial_read;
@@ -245,12 +244,18 @@ TEST_CASE("WebSocket Transport Connection", "[success]")
esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0); esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0);
mock_destroy_ExpectAnyArgsAndReturn(ESP_OK); 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<char>(~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); 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 // 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(std::string(response_header_buffer.data()) == expected_header);
REQUIRE(response_header_buffer[ws_config.response_headers_len] == marker);
} }
} }

4
components/tcp_transport/transport_ws.c
View File

@@ -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) {
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); 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; header_len = ws->response_header_len;
} }
// Copy response header to the static array // Copy response header to the static array
strncpy(ws->response_header, ws->buffer, header_len); 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); char* delim_ptr = strstr(ws->buffer, delimiter);