Skip to content

Harden API and WebSocket receive paths #1845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
skot wants to merge 1 commit into
base: master
Choose a base branch
from
+65 −50
Draft

Harden API and WebSocket receive paths #1845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 47 additions & 40 deletions main/http_server/http_server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,30 @@ static int system_statistics_prebuffer_len = 256;
static int system_wifi_scan_prebuffer_len = 256;
static int api_common_prebuffer_len = 256;

esp_err_t HTTP_receive_body(httpd_req_t *req, char *buffer, size_t buffer_size)

@johnny9 johnny9 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage: the new shared body reader and WebSocket cap have no automated tests. In particular, the exact buffer boundary, fragmented positive reads, zero/negative/over-read failures, and 1024/1025-byte WebSocket boundary are unprotected. The proposal reply extracts the decisions into a small production-used api_rx policy component and covers all of those branches in QEMU.

@johnny9 johnny9 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed coverage patch (apply after the strict-JSON patch above). It passes 77/77 ESP32-S3 QEMU tests and a fresh full ESP-IDF 5.5.3 firmware build:

diff --git a/components/api_rx/CMakeLists.txt b/components/api_rx/CMakeLists.txt
new file mode 100644
--- /dev/null
+++ b/components/api_rx/CMakeLists.txt
@@ -0,0 +1,4 @@
+idf_component_register(
+    SRCS "api_rx.c"
+    INCLUDE_DIRS "include"
+)
diff --git a/components/api_rx/include/api_rx.h b/components/api_rx/include/api_rx.h
new file mode 100644
--- /dev/null
+++ b/components/api_rx/include/api_rx.h
@@ -0,0 +1,21 @@
+#ifndef API_RX_H_
+#define API_RX_H_
+
+#include <stdbool.h>
+#include <stddef.h>
+
+#define API_RX_MAX_WEBSOCKET_PAYLOAD_SIZE 1024U
+
+typedef enum {
+    API_RX_BODY_READ_CONTINUE,
+    API_RX_BODY_READ_COMPLETE,
+    API_RX_BODY_READ_INVALID,
+} api_rx_body_read_result_t;
+
+bool api_rx_http_body_fits(size_t content_len, size_t buffer_size);
+api_rx_body_read_result_t api_rx_body_read_update(size_t content_len,
+                                                  size_t *received_total,
+                                                  int received);
+bool api_rx_websocket_payload_fits(size_t payload_len);
+
+#endif /* API_RX_H_ */
diff --git a/components/api_rx/api_rx.c b/components/api_rx/api_rx.c
new file mode 100644
--- /dev/null
+++ b/components/api_rx/api_rx.c
@@ -0,0 +1,30 @@
+#include "api_rx.h"
+
+bool api_rx_http_body_fits(size_t content_len, size_t buffer_size)
+{
+    return content_len > 0 && content_len < buffer_size;
+}
+
+api_rx_body_read_result_t api_rx_body_read_update(size_t content_len,
+                                                  size_t *received_total,
+                                                  int received)
+{
+    if (received_total == NULL || *received_total > content_len ||
+        received <= 0) {
+        return API_RX_BODY_READ_INVALID;
+    }
+
+    size_t received_size = (size_t)received;
+    if (received_size > content_len - *received_total) {
+        return API_RX_BODY_READ_INVALID;
+    }
+
+    *received_total += received_size;
+    return *received_total == content_len ? API_RX_BODY_READ_COMPLETE
+                                          : API_RX_BODY_READ_CONTINUE;
+}
+
+bool api_rx_websocket_payload_fits(size_t payload_len)
+{
+    return payload_len <= API_RX_MAX_WEBSOCKET_PAYLOAD_SIZE;
+}
diff --git a/components/api_rx/test/CMakeLists.txt b/components/api_rx/test/CMakeLists.txt
new file mode 100644
--- /dev/null
+++ b/components/api_rx/test/CMakeLists.txt
@@ -0,0 +1,5 @@
+idf_component_register(
+    SRCS "test_api_rx.c"
+    INCLUDE_DIRS "."
+    REQUIRES unity api_rx
+)
diff --git a/components/api_rx/test/test_api_rx.c b/components/api_rx/test/test_api_rx.c
new file mode 100644
--- /dev/null
+++ b/components/api_rx/test/test_api_rx.c
@@ -0,0 +1,48 @@
+#include <stdint.h>
+
+#include "api_rx.h"
+#include "unity.h"
+
+TEST_CASE("HTTP body size policy preserves terminator space", "[api_rx]")
+{
+    TEST_ASSERT_FALSE(api_rx_http_body_fits(0, 16));
+    TEST_ASSERT_TRUE(api_rx_http_body_fits(15, 16));
+    TEST_ASSERT_FALSE(api_rx_http_body_fits(16, 16));
+    TEST_ASSERT_FALSE(api_rx_http_body_fits(SIZE_MAX, 16));
+    TEST_ASSERT_FALSE(api_rx_http_body_fits(1, 0));
+}
+
+TEST_CASE("HTTP body read policy handles fragments and failures", "[api_rx]")
+{
+    size_t received_total = 0;
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_CONTINUE,
+        api_rx_body_read_update(5, &received_total, 2));
+    TEST_ASSERT_EQUAL_size_t(2, received_total);
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_COMPLETE,
+        api_rx_body_read_update(5, &received_total, 3));
+    TEST_ASSERT_EQUAL_size_t(5, received_total);
+
+    received_total = 0;
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_INVALID,
+        api_rx_body_read_update(5, &received_total, 0));
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_INVALID,
+        api_rx_body_read_update(5, &received_total, -1));
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_INVALID,
+        api_rx_body_read_update(5, &received_total, 6));
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_INVALID,
+        api_rx_body_read_update(5, NULL, 1));
+
+    received_total = 6;
+    TEST_ASSERT_EQUAL_INT(API_RX_BODY_READ_INVALID,
+        api_rx_body_read_update(5, &received_total, 1));
+}
+
+TEST_CASE("WebSocket payload limit has strict boundary", "[api_rx]")
+{
+    TEST_ASSERT_TRUE(api_rx_websocket_payload_fits(0));
+    TEST_ASSERT_TRUE(api_rx_websocket_payload_fits(
+        API_RX_MAX_WEBSOCKET_PAYLOAD_SIZE));
+    TEST_ASSERT_FALSE(api_rx_websocket_payload_fits(
+        API_RX_MAX_WEBSOCKET_PAYLOAD_SIZE + 1U));
+    TEST_ASSERT_FALSE(api_rx_websocket_payload_fits(SIZE_MAX));
+}
diff --git a/main/CMakeLists.txt b/main/CMakeLists.txt
index 6372448..32258d5 100755
--- a/main/CMakeLists.txt
+++ b/main/CMakeLists.txt
@@ -89,6 +89,7 @@ PRIV_REQUIRES
     "vfs"
     "esp_driver_i2c"
     "tcp_transport"
+    "api_rx"
     "esp_mm"
 
 EMBED_FILES "http_server/recovery_page.html"
diff --git a/main/http_server/http_server.c b/main/http_server/http_server.c
index 1790342..c25edb0 100644
--- a/main/http_server/http_server.c
+++ b/main/http_server/http_server.c
@@ -45,6 +45,7 @@
 #include "log_buffer.h"
 #include "cjson_utils.h"
 #include "utils.h"
+#include "api_rx.h"
 
 static const char * TAG = "http_server";
 static const char * CORS_TAG = "CORS";
@@ -81,17 +82,18 @@ esp_err_t HTTP_receive_body(httpd_req_t *req, char *buffer, size_t buffer_size)
     }
 
     const size_t content_len = req->content_len;
-    if (content_len == 0 || content_len >= buffer_size) {
+    if (!api_rx_http_body_fits(content_len, buffer_size)) {
         return ESP_ERR_INVALID_SIZE;
     }
 
     size_t received_len = 0;
     while (received_len < content_len) {
         int received = httpd_req_recv(req, buffer + received_len, content_len - received_len);
-        if (received <= 0) {
+        api_rx_body_read_result_t result = api_rx_body_read_update(
+            content_len, &received_len, received);
+        if (result == API_RX_BODY_READ_INVALID) {
             return ESP_FAIL;
         }
-        received_len += (size_t)received;
     }
 
     buffer[received_len] = '\0';
diff --git a/main/http_server/websocket.c b/main/http_server/websocket.c
index b473299..f1b70fa 100644
--- a/main/http_server/websocket.c
+++ b/main/http_server/websocket.c
@@ -10,9 +10,9 @@
 #include "websocket_api.h"
 #include "http_server.h"
 #include "log_buffer.h"
+#include "api_rx.h"
 
 #define WS_LOG_SCRATCH_SIZE 2048
-#define WS_RX_MAX_PAYLOAD_SIZE 1024
 
 static const char * TAG = "websocket";
 
@@ -220,12 +220,12 @@ esp_err_t websocket_handler(httpd_req_t *req)
     // WebSocket stream synchronized. Never allocate based on a peer-provided
     // frame length.
     if (ws_pkt.len > 0) {
-        if (ws_pkt.len > WS_RX_MAX_PAYLOAD_SIZE) {
+        if (!api_rx_websocket_payload_fits(ws_pkt.len)) {
             ESP_LOGW(TAG, "Rejecting oversized WebSocket frame: %zu bytes", ws_pkt.len);
             return ESP_ERR_INVALID_SIZE;
         }
 
-        uint8_t buf[WS_RX_MAX_PAYLOAD_SIZE];
+        uint8_t buf[API_RX_MAX_WEBSOCKET_PAYLOAD_SIZE];
         ws_pkt.payload = buf;
         return httpd_ws_recv_frame(req, &ws_pkt, sizeof(buf));
     }
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index e98ced1..d972cfb 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -10,7 +10,7 @@ set(EXTRA_COMPONENT_DIRS "../components")
 # - when invoking CMake directly: cmake -D TEST_COMPONENTS="xxxxx" ..
 # - when using idf.py: idf.py -T xxxxx build
 #
-set(TEST_COMPONENTS "stratum asic" CACHE STRING "List of components to test")
+set(TEST_COMPONENTS "stratum asic api_rx" CACHE STRING "List of components to test")
 
 include($ENV{IDF_PATH}/tools/cmake/project.cmake)

{
if (req == NULL || buffer == NULL || buffer_size == 0) {
return ESP_ERR_INVALID_ARG;
}

const size_t content_len = req->content_len;
if (content_len == 0 || content_len >= buffer_size) {
return ESP_ERR_INVALID_SIZE;
}

size_t received_len = 0;
while (received_len < content_len) {
int received = httpd_req_recv(req, buffer + received_len, content_len - received_len);
if (received <= 0) {
return ESP_FAIL;
}
received_len += (size_t)received;
}

buffer[received_len] = '\0';
return ESP_OK;
}

typedef enum
{
SRC_HASHRATE,
Expand Down Expand Up @@ -1043,25 +1067,16 @@ static esp_err_t PATCH_update_settings(httpd_req_t * req)
return ESP_OK;
}

int total_len = req->content_len;
int cur_len = 0;
char * buf = ((rest_server_context_t *) (req->user_ctx))->scratch;
int received = 0;
if (total_len >= SCRATCH_BUFSIZE) {
/* Respond with 500 Internal Server Error */
httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "content too long");
return ESP_OK;
esp_err_t receive_result = HTTP_receive_body(req, buf, SCRATCH_BUFSIZE);

@johnny9 johnny9 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: HTTP_receive_body now supplies a correctly terminated buffer, but the handlers pass it to cJSON_Parse, which accepts a valid first JSON value followed by arbitrary non-whitespace bytes. Settings, pool, boot, and theme requests can therefore accept malformed documents such as {...} trailing. The proposal reply makes all four request parsers require the complete body.

@johnny9 johnny9 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed patch. It applies to the exact reviewed head; it was compile-validated in the full ESP-IDF 5.5.3 firmware build and included in the combined validation:

diff --git a/main/http_server/http_server.c b/main/http_server/http_server.c
index a789d35..7ca78f4 100644
--- a/main/http_server/http_server.c
+++ b/main/http_server/http_server.c
@@ -1078,7 +1078,7 @@ static esp_err_t PATCH_update_settings(httpd_req_t * req)
         return ESP_FAIL;
     }
 
-    cJSON * root = cJSON_Parse(buf);
+    cJSON * root = cJSON_ParseWithOpts(buf, NULL, true);
     if (root == NULL) {
         httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid JSON");
         return ESP_OK;
@@ -1272,7 +1272,7 @@ static esp_err_t PUT_system_pool(httpd_req_t *req)
         return ESP_FAIL;
     }
 
-    cJSON *root = cJSON_Parse(buf);
+    cJSON *root = cJSON_ParseWithOpts(buf, NULL, true);
     if (!root) {
         return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid JSON");
     }
@@ -1453,7 +1453,7 @@ static esp_err_t POST_system_boot(httpd_req_t *req)
         return ESP_FAIL;
     }
 
-    cJSON *root = cJSON_Parse(buf);
+    cJSON *root = cJSON_ParseWithOpts(buf, NULL, true);
     if (!root) {
         return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid JSON");
     }
diff --git a/main/http_server/theme_api.c b/main/http_server/theme_api.c
index 7878ee8..f2d0b0d 100644
--- a/main/http_server/theme_api.c
+++ b/main/http_server/theme_api.c
@@ -57,7 +57,7 @@ static esp_err_t theme_post_handler(httpd_req_t *req)
         return ESP_FAIL;
     }
 
-    cJSON *root = cJSON_Parse(content);
+    cJSON *root = cJSON_ParseWithOpts(content, NULL, true);
     if (!root) {
         httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid JSON");
         return ESP_FAIL;

if (receive_result == ESP_ERR_INVALID_SIZE) {
httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid request length");
return ESP_FAIL;
}
while (cur_len < total_len) {
received = httpd_req_recv(req, buf + cur_len, total_len);
if (received <= 0) {
/* Respond with 500 Internal Server Error */
httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to post control value");
return ESP_OK;
}
cur_len += received;
if (receive_result != ESP_OK) {
httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to receive request data");
return ESP_FAIL;
}
buf[total_len] = '\0';

cJSON * root = cJSON_Parse(buf);
if (root == NULL) {
Expand Down Expand Up @@ -1246,17 +1261,16 @@ static esp_err_t PUT_system_pool(httpd_req_t *req)
return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid pool index");
}

int total_len = req->content_len;
if (total_len <= 0 || total_len >= SCRATCH_BUFSIZE) {
return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid request length");
}

char *buf = ((rest_server_context_t *)(req->user_ctx))->scratch;
int received = httpd_req_recv(req, buf, total_len);
if (received <= 0) {
return httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to receive request data");
esp_err_t receive_result = HTTP_receive_body(req, buf, SCRATCH_BUFSIZE);
if (receive_result == ESP_ERR_INVALID_SIZE) {
httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid request length");
return ESP_FAIL;
}
if (receive_result != ESP_OK) {
httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to receive request data");
return ESP_FAIL;
}
buf[received] = '\0';

cJSON *root = cJSON_Parse(buf);
if (!root) {
Expand Down Expand Up @@ -1428,25 +1442,18 @@ static esp_err_t POST_system_boot(httpd_req_t *req)
return ESP_OK;
}

size_t total_len = req->content_len;
if (total_len == 0) {
return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Empty request body");
}

char *buf = malloc(total_len + 1);
if (!buf) {
return httpd_resp_send_500(req);
char *buf = ((rest_server_context_t *)(req->user_ctx))->scratch;
esp_err_t receive_result = HTTP_receive_body(req, buf, SCRATCH_BUFSIZE);
if (receive_result == ESP_ERR_INVALID_SIZE) {
httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid request length");
return ESP_FAIL;
}

int ret = httpd_req_recv(req, buf, total_len);
if (ret <= 0) {
free(buf);
return httpd_resp_send_500(req);
if (receive_result != ESP_OK) {
httpd_resp_send_500(req);
return ESP_FAIL;
}
buf[ret] = '\0';

cJSON *root = cJSON_Parse(buf);
free(buf);
if (!root) {
return httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid JSON");
}
Expand Down
1 change: 1 addition & 0 deletions main/http_server/http_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ esp_err_t is_network_allowed(httpd_req_t * req);
esp_err_t set_cors_headers(httpd_req_t * req);
esp_err_t start_rest_server(GlobalState * GLOBAL_STATE);
esp_err_t HTTP_send_json(httpd_req_t * req, const cJSON * item, int * prebuffer_len);
esp_err_t HTTP_receive_body(httpd_req_t *req, char *buffer, size_t buffer_size);

#endif /* HTTP_SERVER_H_ */
9 changes: 6 additions & 3 deletions main/http_server/theme_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,15 @@ static esp_err_t theme_post_handler(httpd_req_t *req)

// Read POST data
char content[1024];
int ret = httpd_req_recv(req, content, sizeof(content) - 1);
if (ret <= 0) {
esp_err_t receive_result = HTTP_receive_body(req, content, sizeof(content));
if (receive_result == ESP_ERR_INVALID_SIZE) {
httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Invalid request length");
return ESP_FAIL;
}
if (receive_result != ESP_OK) {
httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Failed to read request");
return ESP_FAIL;
}
content[ret] = '\0';

cJSON *root = cJSON_Parse(content);
if (!root) {
Expand Down
18 changes: 11 additions & 7 deletions main/http_server/websocket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "log_buffer.h"

#define WS_LOG_SCRATCH_SIZE 2048
#define WS_RX_MAX_PAYLOAD_SIZE 1024

static const char * TAG = "websocket";

Expand Down Expand Up @@ -215,15 +216,18 @@ esp_err_t websocket_handler(httpd_req_t *req)
return ret;
}

// If there's a payload, drain it
// Inbound application data is ignored, but it must be drained to keep the
// WebSocket stream synchronized. Never allocate based on a peer-provided
// frame length.
if (ws_pkt.len > 0) {
uint8_t *buf = (uint8_t *)calloc(1, ws_pkt.len + 1);
if (buf) {
ws_pkt.payload = buf;
ret = httpd_ws_recv_frame(req, &ws_pkt, ws_pkt.len);
free(buf);
return ret;
if (ws_pkt.len > WS_RX_MAX_PAYLOAD_SIZE) {
ESP_LOGW(TAG, "Rejecting oversized WebSocket frame: %zu bytes", ws_pkt.len);
return ESP_ERR_INVALID_SIZE;
}

uint8_t buf[WS_RX_MAX_PAYLOAD_SIZE];
ws_pkt.payload = buf;
return httpd_ws_recv_frame(req, &ws_pkt, sizeof(buf));
}

return ESP_OK;
Expand Down
Loading