diff --git a/components/esp_http_server/include/esp_http_server.h b/components/esp_http_server/include/esp_http_server.h index bcebe2dd1aa..1ca768ae257 100644 --- a/components/esp_http_server/include/esp_http_server.h +++ b/components/esp_http_server/include/esp_http_server.h @@ -689,8 +689,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 diff --git a/components/esp_http_server/src/httpd_parse.c b/components/esp_http_server/src/httpd_parse.c index a5be0cced71..8245ae3291a 100644 --- a/components/esp_http_server/src/httpd_parse.c +++ b/components/esp_http_server/src/httpd_parse.c @@ -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; @@ -372,6 +374,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 diff --git a/components/esp_http_server/src/httpd_txrx.c b/components/esp_http_server/src/httpd_txrx.c index ae2b42e8be4..cb630696265 100644 --- a/components/esp_http_server/src/httpd_txrx.c +++ b/components/esp_http_server/src/httpd_txrx.c @@ -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