Skip to content

Commit 23572e0

Browse files
author
glmfe
committed
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
1 parent a319aa9 commit 23572e0

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

components/tcp_transport/host_test/main/test_websocket_transport.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ TEST_CASE("WebSocket Transport Connection", "[success]")
220220
"Sec-WebSocket-Accept:\r\n"
221221
"\r\n";
222222
REQUIRE(std::string(response_header_buffer.data()) == expected_header);
223-
224223
char buffer[WS_BUFFER_SIZE];
225224
int read_len = 0;
226225
int partial_read;
@@ -245,12 +244,18 @@ TEST_CASE("WebSocket Transport Connection", "[success]")
245244
esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0);
246245
mock_destroy_ExpectAnyArgsAndReturn(ESP_OK);
247246

247+
// Create a marker to check that the value after the end of the response header buffer is not overwritten
248+
std::string expected_full_header = make_response();
249+
char marker = static_cast<char>(~expected_full_header[ws_config.response_headers_len]);
250+
response_header_buffer[ws_config.response_headers_len] = marker;
251+
248252
REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0);
249253

250254
// Verify the response header was stored correctly. it must contain only ten bytes and be null terminated
251-
std::string expected_header = "HTTP/1.1 1\0";
255+
std::string expected_header = "HTTP/1.1 \0";
252256

253257
REQUIRE(std::string(response_header_buffer.data()) == expected_header);
258+
REQUIRE(response_header_buffer[ws_config.response_headers_len] == marker);
254259
}
255260
}
256261

components/tcp_transport/transport_ws.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,13 +312,13 @@ static int ws_connect(esp_transport_handle_t t, const char *host, int port, int
312312
}
313313

314314
if(ws->response_header) {
315-
if(ws->response_header_len < header_len) {
315+
if(ws->response_header_len - 1 < header_len) {
316316
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);
317317
header_len = ws->response_header_len;
318318
}
319319
// Copy response header to the static array
320320
strncpy(ws->response_header, ws->buffer, header_len);
321-
ws->response_header[header_len] = '\0';
321+
ws->response_header[ws->response_header_len - 1] = '\0';
322322
}
323323

324324
char* delim_ptr = strstr(ws->buffer, delimiter);

0 commit comments

Comments
 (0)