Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-10-01 10:40:47 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'fix/http_server_chunked_501' into 'master'

Browse Source 
Fix(esp_http_server): Fail cleanly on incoming chunked request

See merge request espressif/esp-idf!52126
This commit is contained in:
Mahavir Jain
2026-09-06 15:55:17 +05:30
parent a482934bff 036cd477a3
commit 96f54947e0
4 changed files with 225 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/esp_http_server/include/esp_http_server.h
+12 -9
View File
@@ -380,7 +380,7 @@ esp_err_t httpd_stop(httpd_handle_t handle);
*/
typedef struct httpd_req {
httpd_handle_t handle; /*!< Handle to server instance */
int method; /*!< The type of HTTP request, -1 if unsupported method, HTTP_ANY for wildcard method to support every method */
int method; /*!< The type of HTTP request (enum http_method); a request with an unrecognized method is rejected with 501 before any URI handler runs */
const char uri[CONFIG_HTTPD_MAX_URI_LEN + 1]; /*!< The URI of this request (1 byte extra for null termination) */
size_t content_len; /*!< Length of the request body */
void *aux; /*!< Internally used members */
@@ -623,10 +623,11 @@ typedef enum {
*/
HTTPD_500_INTERNAL_SERVER_ERROR = 0,
/* For methods not supported by http_parser. Presently
* http_parser halts parsing when such methods are
* encountered and so the server responds with 400 Bad
* Request error instead.
/* For request methods that http_parser does not recognize,
* and for requests carrying a Transfer-Encoding header (no
* transfer coding is implemented). Parsing aborted mid-request
* in both cases, so the server closes the session after the
* response regardless of what a custom error handler returns.
*/
HTTPD_501_METHOD_NOT_IMPLEMENTED,
@@ -634,8 +635,8 @@ typedef enum {
HTTPD_505_VERSION_NOT_SUPPORTED,
/* Returned when http_parser halts parsing due to incorrect
* syntax of request, unsupported method in request URI or
* due to chunked encoding / upgrade field present in headers
* syntax of request or due to an upgrade field present in
* headers that the server does not handle
*/
HTTPD_400_BAD_REQUEST,
@@ -700,8 +701,10 @@ typedef enum {
* guaranteed as the HTTP request may be partially received/parsed.
* - The function must return ESP_OK if underlying socket needs to
* be kept open. Any other value will ensure that the socket is
* closed. The return value is ignored when error is of type
* `HTTPD_500_INTERNAL_SERVER_ERROR` and the socket closed anyway.
* closed. The return value is ignored, and the socket is closed
* anyway, when the error is `HTTPD_500_INTERNAL_SERVER_ERROR` or
* `HTTPD_501_METHOD_NOT_IMPLEMENTED` (for 501 the request is only
* partially parsed, so the session cannot continue safely).
*
* @param[in] req HTTP request for which the error needs to be handled
* @param[in] error Error type
components/esp_http_server/src/httpd_parse.c
+33 -7
View File
@@ -55,6 +55,8 @@ typedef struct {
size_t raw_datalen; /*!< Full length of the raw data in scratch buffer */
} parser_data_t;
static const char *httpd_find_hdr_value(struct httpd_req_aux *ra, const char *field);
static esp_err_t verify_url (http_parser *parser)
{
parser_data_t *parser_data = (parser_data_t *) parser->data;
@@ -66,12 +68,10 @@ static esp_err_t verify_url (http_parser *parser)
const char *at = parser_data->last.at;
size_t length = parser_data->last.length;
/* http_parser stops with HPE_INVALID_METHOD before the URL callback for
* any method token it does not know, so the method is always valid here.
* That parser error is mapped to 501 in parse_block(). */
r->method = parser->method;
if (r->method < 0) {
ESP_LOGW(TAG, LOG_FMT("HTTP method not supported (%d)"), r->method);
parser_data->error = HTTPD_501_METHOD_NOT_IMPLEMENTED;
return ESP_FAIL;
}
if (sizeof(r->uri) < (length + 1)) {
ESP_LOGW(TAG, LOG_FMT("URI length (%"NEWLIB_NANO_COMPAT_FORMAT") greater than supported (%"NEWLIB_NANO_COMPAT_FORMAT")"),
@@ -372,6 +372,21 @@ static esp_err_t cb_headers_complete(http_parser *parser)
return ESP_FAIL;
}
/* This server implements no transfer codings, so reject any request that
* carries a Transfer-Encoding header with 501 (RFC 9112 section 6.1).
* Without this check a chunked body is treated as an empty body and its
* chunk framing is parsed as a separate pipelined request, which is a
* request-smuggling primitive when the server sits behind a proxy. The
* header-presence check also covers codings that http_parser does not
* flag as chunked (e.g. "gzip, chunked"). */
if ((parser->flags & F_CHUNKED) ||
httpd_find_hdr_value(ra, "Transfer-Encoding") != NULL) {
ESP_LOGW(TAG, LOG_FMT("Transfer-Encoding is not supported"));
parser_data->error = HTTPD_501_METHOD_NOT_IMPLEMENTED;
parser_data->status = PARSING_FAILED;
return ESP_FAIL;
}
/* In absence of body/chunked encoding, http_parser sets content_len to ULLONG_MAX */
if (parser->content_length != ULLONG_MAX) {
/* Content-Length was specified. Reject any value above UINT32_MAX: it is
@@ -583,6 +598,17 @@ static int read_block(httpd_req_t *req, http_parser *parser, size_t offset, size
return nbytes;
}
/* Map an http_parser failure to the HTTP status sent back. An unrecognized
* request method is a 501 (RFC 9110 section 15.6.2), every other parser
* error means the request syntax is malformed. */
static httpd_err_code_t parser_errno_to_err_code(const http_parser *parser)
{
if (HTTP_PARSER_ERRNO(parser) == HPE_INVALID_METHOD) {
return HTTPD_501_METHOD_NOT_IMPLEMENTED;
}
return HTTPD_400_BAD_REQUEST;
}
static int parse_block(http_parser *parser, size_t offset, size_t length)
{
parser_data_t *data = (parser_data_t *)(parser->data);
@@ -644,14 +670,14 @@ static int parse_block(http_parser *parser, size_t offset, size_t length)
return 0;
} else if (nparsed != length) {
/* http_parser error */
data->error = HTTPD_400_BAD_REQUEST;
data->error = parser_errno_to_err_code(parser);
data->status = PARSING_FAILED;
ESP_LOGW(TAG, LOG_FMT("incomplete (%"NEWLIB_NANO_COMPAT_FORMAT"/%"NEWLIB_NANO_COMPAT_FORMAT") with parser error = %d"),
NEWLIB_NANO_COMPAT_CAST(nparsed), NEWLIB_NANO_COMPAT_CAST(length), parser->http_errno);
return -1;
} else if (HTTP_PARSER_ERRNO(parser) != HPE_OK) {
/* http_parser error */
data->error = HTTPD_400_BAD_REQUEST;
data->error = parser_errno_to_err_code(parser);
data->status = PARSING_FAILED;
ESP_LOGE(TAG, LOG_FMT("parser error: %s"), http_errno_description(HTTP_PARSER_ERRNO(parser)));
return -1;
components/esp_http_server/src/httpd_txrx.c
+10 -3
View File
@@ -603,9 +603,16 @@ esp_err_t httpd_req_handle_err(httpd_req_t *req, httpd_err_code_t error)
if (hd->err_handler_fns[error]) {
ret = hd->err_handler_fns[error](req, error);
/* If error code is 500, force return failure
* irrespective of the handler's return value */
ret = (error == HTTPD_500_INTERNAL_SERVER_ERROR ? ESP_FAIL : ret);
/* Force failure irrespective of the handler's return value for
* errors after which the session cannot continue safely: 500
* (internal state is broken) and 501 (parsing aborted mid-request,
* so the position of the next request in the byte stream is
* unknown and keep-alive would let leftover bytes be parsed and
* answered as a further request). */
if (error == HTTPD_500_INTERNAL_SERVER_ERROR ||
error == HTTPD_501_METHOD_NOT_IMPLEMENTED) {
ret = ESP_FAIL;
}
} else {
/* If no handler is registered for this error default
* behavior is to send the HTTP error response and
components/esp_http_server/test_apps/main/test_http_server.c
+170
View File
@@ -570,6 +570,176 @@ TEST_CASE("httpd_queue_work fast-fails on ctrl mbox saturation", "[HTTP SERVER]"
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
/* ---- Transfer-Encoding rejection (RFC 9112 section 6.1) ---- */
/* The server implements no transfer codings, so any request carrying a
* Transfer-Encoding header must be answered with 501. Without the parser
* check, a chunked body is misread as an empty body and its chunk framing
* is then parsed as a separate pipelined request — a request-smuggling
* primitive when the server sits behind a proxy. */
static httpd_handle_t start_plain_test_server(uint16_t server_port, uint16_t ctrl_port)
{
httpd_handle_t hd = NULL;
httpd_config_t config = HTTPD_DEFAULT_CONFIG();
config.server_port = server_port;
config.ctrl_port = ctrl_port;
TEST_ASSERT_EQUAL(ESP_OK, httpd_start(&hd, &config));
return hd;
}
static esp_err_t te_ok_handler(httpd_req_t *req)
{
return httpd_resp_sendstr(req, "OK");
}
/* Assert the captured wire data holds exactly one HTTP response and that the
* server closed the session afterwards. This is what proves the bytes after
* the rejected framing were NOT parsed and answered as a further request. */
static void assert_single_response_then_close(const mock_server_response_t *resp)
{
TEST_ASSERT_TRUE_MESSAGE(resp->len > 0,
"no response received before session close");
TEST_ASSERT_TRUE_MESSAGE(resp->server_closed,
"server must close the session after 501");
TEST_ASSERT_NULL_MESSAGE(strstr(resp->data + 1, "HTTP/1."),
"trailing bytes were answered as a second request");
}
TEST_CASE("Chunked request is rejected with 501", "[HTTP SERVER][security]")
{
test_case_uses_tcpip();
httpd_handle_t hd = start_plain_test_server(8098, ESP_HTTPD_DEF_CTRL_PORT + 17);
/* Register real handlers: without the parser-level reject, "POST /any"
* would succeed with an empty body (keep-alive stays up) and the chunk
* framing plus the pipelined GET would be parsed as further requests,
* producing a second response on the same connection. */
httpd_uri_t post_any = {
.uri = "/any", .method = HTTP_POST, .handler = te_ok_handler,
};
httpd_uri_t get_smuggled = {
.uri = "/smuggled", .method = HTTP_GET, .handler = te_ok_handler,
};
TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &post_any));
TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &get_smuggled));
mock_server_request_t req = {
.data = "POST /any HTTP/1.1\r\n"
"Host: localhost\r\n"
"Transfer-Encoding: chunked\r\n"
"\r\n"
"5\r\nhello\r\n0\r\n\r\n"
"GET /smuggled HTTP/1.1\r\n"
"Host: localhost\r\n"
"\r\n",
};
mock_server_response_t *resp = mock_server_send_request(8098, &req);
TEST_ASSERT_NOT_NULL(resp);
assert_single_response_then_close(resp);
mock_server_assert_status(resp, 501);
mock_server_response_free(resp);
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
static esp_err_t err_501_keepalive_handler(httpd_req_t *req, httpd_err_code_t error)
{
httpd_resp_send_err(req, error, NULL);
/* Try to keep the session open. The server must ignore this for 501:
* parsing aborted mid-request, so the stream has no safe continuation. */
return ESP_OK;
}
TEST_CASE("Custom 501 handler cannot keep the session open", "[HTTP SERVER][security]")
{
test_case_uses_tcpip();
httpd_handle_t hd = start_plain_test_server(8100, ESP_HTTPD_DEF_CTRL_PORT + 19);
TEST_ASSERT_EQUAL(ESP_OK, httpd_register_err_handler(hd, HTTPD_501_METHOD_NOT_IMPLEMENTED,
err_501_keepalive_handler));
mock_server_request_t req = {
.data = "POST /any HTTP/1.1\r\n"
"Host: localhost\r\n"
"Transfer-Encoding: chunked\r\n"
"\r\n"
"5\r\nhello\r\n0\r\n\r\n",
};
mock_server_response_t *resp = mock_server_send_request(8100, &req);
TEST_ASSERT_NOT_NULL(resp);
assert_single_response_then_close(resp);
mock_server_assert_status(resp, 501);
mock_server_response_free(resp);
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
TEST_CASE("Unknown Transfer-Encoding is rejected with 501", "[HTTP SERVER][security]")
{
test_case_uses_tcpip();
httpd_handle_t hd = start_plain_test_server(8099, ESP_HTTPD_DEF_CTRL_PORT + 18);
/* "gzip" does not set the parser's chunked flag; presence of the header
* alone must trigger the 501 so the body cannot be silently skipped. */
mock_server_request_t req = {
.data = "POST /any HTTP/1.1\r\n"
"Host: localhost\r\n"
"Transfer-Encoding: gzip\r\n"
"Content-Length: 5\r\n"
"\r\n"
"hello",
};
mock_server_response_t *resp = mock_server_send_request(8099, &req);
TEST_ASSERT_NOT_NULL(resp);
assert_single_response_then_close(resp);
mock_server_assert_status(resp, 501);
mock_server_response_free(resp);
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
/* ---- Unrecognized request method (RFC 9110 section 15.6.2) ---- */
/* http_parser stops on a method token it does not know, before the URL
* callback ever runs. The server must map that parser error to 501, not to
* the generic 400 used for malformed syntax, and must close the session:
* parsing aborted mid-request, so the stream has no safe continuation. */
TEST_CASE("Unknown HTTP method is rejected with 501", "[HTTP SERVER]")
{
test_case_uses_tcpip();
httpd_handle_t hd = start_plain_test_server(8101, ESP_HTTPD_DEF_CTRL_PORT + 20);
httpd_uri_t get_any = {
.uri = "/any", .method = HTTP_GET, .handler = te_ok_handler,
};
TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &get_any));
/* "FOO" is a syntactically valid token that no server implements. The
* pipelined GET behind it must never be answered. */
mock_server_request_t req = {
.data = "FOO /any HTTP/1.1\r\n"
"Host: localhost\r\n"
"\r\n"
"GET /any HTTP/1.1\r\n"
"Host: localhost\r\n"
"\r\n",
};
mock_server_response_t *resp = mock_server_send_request(8101, &req);
TEST_ASSERT_NOT_NULL(resp);
assert_single_response_then_close(resp);
mock_server_assert_status(resp, 501);
mock_server_response_free(resp);
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
/* Guard: only the unknown-method parser error maps to 501. Any other
* request-line syntax error must still be answered with 400. */
TEST_CASE("Malformed request line is rejected with 400", "[HTTP SERVER]")
{
test_case_uses_tcpip();
httpd_handle_t hd = start_plain_test_server(8102, ESP_HTTPD_DEF_CTRL_PORT + 21);
mock_server_request_t req = {
.data = "GET /any HTP/1.1\r\n"
"Host: localhost\r\n"
"\r\n",
};
mock_server_response_t *resp = mock_server_send_request(8102, &req);
TEST_ASSERT_NOT_NULL(resp);
mock_server_assert_status(resp, 400);
mock_server_response_free(resp);
TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd));
}
#ifdef CONFIG_HTTPD_WS_SUPPORT
/* ------------------------------------------------------------------------- *
* White-box fixtures for the dedicated control-frame handler.