feedc0de/esp-idf
1
0
Fork 0
forked from espressif/esp-idf
Code Pull Requests Activity

Merge branch 'fix/ws_transport_mem_corrupt' into 'master'

Browse Source 
fix(transport): Fix websocket mem-corruption while reading headers

Closes IDFGH-13585

See merge request espressif/esp-idf!33593
This commit is contained in:
David Čermák
2024-09-26 18:53:00 +08:00
parent bcfa928850 53e63eb125
commit 4f467d02c7
2 changed files with 46 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -21,7 +21,7 @@
#include "esp_transport_tcp.h" #include "esp_transport_tcp.h"
#include "esp_transport_ws.h" #include "esp_transport_ws.h"
#define WS_BUFFER_SIZE 1024 #define WS_BUFFER_SIZE CONFIG_WS_BUFFER_SIZE
extern "C" { extern "C" {
#include "Mockmock_transport.h" #include "Mockmock_transport.h"
@ -103,7 +103,8 @@ int mock_write_callback(esp_transport_handle_t transport, const char *request_se
} }
// Callback function for mock_read // Callback function for mock_read
int mock_read_callback(esp_transport_handle_t transport, char *buffer, int len, int timeout_ms, int num_call) { int mock_valid_read_callback(esp_transport_handle_t transport, char *buffer, int len, int timeout_ms, int num_call)
{
std::string websocket_response = make_response(); std::string websocket_response = make_response();
std::memcpy(buffer, websocket_response.data(), websocket_response.size()); std::memcpy(buffer, websocket_response.data(), websocket_response.size());
return websocket_response.size(); return websocket_response.size();
@ -111,7 +112,7 @@ int mock_read_callback(esp_transport_handle_t transport, char *buffer, int len,
} }
TEST_CASE("WebSocket Transport Connection", "[websocket_transport]") { void test_ws_connect(bool expect_valid_connection, CMOCK_mock_read_CALLBACK read_callback) {
constexpr static auto timeout = 50; constexpr static auto timeout = 50;
constexpr static auto port = 8080; constexpr static auto port = 8080;
constexpr static auto host = "localhost"; constexpr static auto host = "localhost";
@ -134,10 +135,18 @@ TEST_CASE("WebSocket Transport Connection", "[websocket_transport]") {
mock_write_Stub(mock_write_callback); mock_write_Stub(mock_write_callback);
mock_connect_ExpectAndReturn(parent_handle.get(), host, port, timeout, ESP_OK); mock_connect_ExpectAndReturn(parent_handle.get(), host, port, timeout, ESP_OK);
// Set the callback function for mock_read // Set the callback function for mock_read
mock_read_Stub(mock_read_callback); mock_read_Stub(read_callback);
mock_poll_read_ExpectAnyArgsAndReturn(1); mock_poll_read_ExpectAnyArgsAndReturn(1);
esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0); esp_crypto_base64_encode_ExpectAnyArgsAndReturn(0);
mock_destroy_ExpectAnyArgsAndReturn(ESP_OK); mock_destroy_ExpectAnyArgsAndReturn(ESP_OK);
if (!expect_valid_connection) {
// for invalid connections we only check that the connect() function fails
REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) != 0);
// and we're done here
return;
}
REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0);
char buffer[WS_BUFFER_SIZE]; char buffer[WS_BUFFER_SIZE];
@ -147,5 +156,37 @@ TEST_CASE("WebSocket Transport Connection", "[websocket_transport]") {
std::string response(buffer, read_len); std::string response(buffer, read_len);
REQUIRE(response == "Test"); REQUIRE(response == "Test");
} }
} }
// Happy flow
TEST_CASE("WebSocket Transport Connection", "[websocket_transport]")
{
test_ws_connect(true, mock_valid_read_callback);
}
// Some corner cases where we expect the ws connection to fail
TEST_CASE("ws connect fails (0 len response)", "[websocket_transport]")
{
test_ws_connect(false, [](esp_transport_handle_t h, char *buf, int len, int tout, int n) {
return 0;
});
}
TEST_CASE("ws connect fails (invalid response)", "[websocket_transport]")
{
test_ws_connect(false, [](esp_transport_handle_t h, char *buf, int len, int tout, int n) {
int resp_len = len/2;
std::memset(buf, '?', resp_len);
return resp_len;
});
}
TEST_CASE("ws connect fails (big response)", "[websocket_transport]")
{
test_ws_connect(false, [](esp_transport_handle_t h, char *buf, int len, int tout, int n) {
return WS_BUFFER_SIZE;
});
}

2
components/tcp_transport/transport_ws.c
View File

@ -285,7 +285,7 @@ static int ws_connect(esp_transport_handle_t t, const char *host, int port, int
} }
int header_len = 0; int header_len = 0;
do { do {
if ((len = esp_transport_read(ws->parent, ws->buffer + header_len, WS_BUFFER_SIZE - header_len, timeout_ms)) <= 0) { if ((len = esp_transport_read(ws->parent, ws->buffer + header_len, WS_BUFFER_SIZE - 1 - header_len, timeout_ms)) <= 0) {
ESP_LOGE(TAG, "Error read response for Upgrade header %s", ws->buffer); ESP_LOGE(TAG, "Error read response for Upgrade header %s", ws->buffer);
return -1; return -1;
} }