Merge branch 'contrib/github_pr_18734_v6.1' into 'release/v6.1'

Fix header value truncation detection in httpd_req_get_hdr_value_str (GitHub PR) (v6.1)

See merge request espressif/esp-idf!50536

components/esp_http_server/include/esp_http_server.h

@@ -1007,6 +1007,33 @@ size_t httpd_req_get_hdr_value_len(httpd_req_t *r, const char *field);
  */
 esp_err_t httpd_req_get_hdr_value_str(httpd_req_t *r, const char *field, char *val, size_t val_size);
 
+/**
+ * @brief   Similar to httpd_req_get_hdr_value_str() but avoids the string copy
+ *
+ * @note
+ *  - This API is meant to be used with std::string_view or similar
+ *  - The returned pointer references the header value kept in the request's
+ *    internal scratch buffer. It is NULL-terminated; val_len is provided for
+ *    convenience (e.g. constructing a std::string_view without a strlen()).
+ *  - This API is supposed to be called only from the context of
+ *    a URI handler where httpd_req_t* request pointer is valid.
+ *  - The returned pointer must only be used within the URI handler and not
+ *    after, since it will lead to use after free errors. In particular, once
+ *    httpd_resp_send() is called all request headers are purged.
+ *
+ * @param[in]  r        The request being responded to
+ * @param[in]  field    The field to be searched in the header
+ * @param[out] val      Pointer to a pointer that will be updated to the value string
+ * @param[out] val_len  Pointer to a length that will be updated with the value length
+ *
+ * @return
+ *  - ESP_OK : Field found; *val points to the value string and *val_len holds its length
+ *  - ESP_ERR_NOT_FOUND          : Key not found
+ *  - ESP_ERR_INVALID_ARG        : Null arguments
+ *  - ESP_ERR_HTTPD_INVALID_REQ  : Invalid HTTP request pointer
+ */
+esp_err_t httpd_req_get_hdr_value_str_ptr(httpd_req_t *r, const char *field, const char **val, size_t *val_len);
+
 /**
  * @brief   Get Query string length from the request URL
  *
@@ -1053,6 +1080,36 @@ size_t httpd_req_get_url_query_len(httpd_req_t *r);
  */
 esp_err_t httpd_req_get_url_query_str(httpd_req_t *r, char *buf, size_t buf_len);
 
+/**
+ * @brief   Similar to httpd_req_get_url_query_str() but avoids the string copy
+ *
+ * @note
+ *  - This API is meant to be used with std::string_view or similar
+ *  - Presently, the user can fetch the full URL query string, but decoding
+ *    will have to be performed by the user. Request headers can be read using
+ *    httpd_req_get_hdr_value_str() to know the 'Content-Type' (eg. Content-Type:
+ *    application/x-www-form-urlencoded) and then the appropriate decoding
+ *    algorithm needs to be applied.
+ *  - This API is supposed to be called only from the context of
+ *    a URI handler where httpd_req_t* request pointer is valid
+ *  - The byte range between buf and buf_len should only be used within
+ *    the URI handler and not after since it will lead to use after free
+ *    errors
+ *
+ * @param[in]  r         The request being responded to
+ * @param[out] buf       Pointer to a pointer that should be updated to the beginning of
+ *                       the query string within the request URL
+ * @param[out] buf_len   Pointer to a length that will be updated with the query length
+ *
+ * @return
+ *  - ESP_OK : Query found; *buf points to the query string and *buf_len holds its length
+ *  - ESP_FAIL                   : uri is empty
+ *  - ESP_ERR_NOT_FOUND          : Query not found
+ *  - ESP_ERR_INVALID_ARG        : Null arguments
+ *  - ESP_ERR_HTTPD_INVALID_REQ  : Invalid HTTP request pointer
+ */
+esp_err_t httpd_req_get_url_query_str_ptr(httpd_req_t *r, const char **buf, size_t *buf_len);
+
 /**
  * @brief   Helper function to get a URL query tag from a query
  *          string of the type param1=val1&param2=val2

components/esp_http_server/src/httpd_parse.c

@@ -1036,6 +1036,92 @@ esp_err_t httpd_req_get_url_query_str(httpd_req_t *r, char *buf, size_t buf_len)
     return ESP_ERR_NOT_FOUND;
 }
 
+esp_err_t httpd_req_get_url_query_str_ptr(httpd_req_t *r, const char **buf, size_t *buf_len)
+{
+    /* buf_len is an output pointer that is dereferenced below, so it must be
+     * validated along with the other arguments to avoid a NULL write */
+    if (r == NULL || buf == NULL || buf_len == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    if (!httpd_valid_req(r)) {
+        return ESP_ERR_HTTPD_INVALID_REQ;
+    }
+
+    if (r->uri[0] == '\0') {
+        ESP_LOGD(TAG, "uri is empty");
+        return ESP_FAIL;
+    }
+
+    struct httpd_req_aux   *ra  = r->aux;
+    struct http_parser_url *res = &ra->url_parse_res;
+
+    /* Check if query field is present in the URL */
+    if (res->field_set & (1 << UF_QUERY)) {
+        *buf = r->uri + res->field_data[UF_QUERY].off;
+
+        /* Query data length does not include terminating null */
+        *buf_len = res->field_data[UF_QUERY].len;
+
+        return ESP_OK;
+    }
+    return ESP_ERR_NOT_FOUND;
+}
+
+/* Advance the scratch pointer past the current header line to the next one.
+ * Header lines are separated by null characters (their line terminators are
+ * overwritten with '\0' during parsing). */
+static const char *httpd_next_hdr(const char *hdr_ptr)
+{
+    /* Jump to the end of the current field-value string */
+    hdr_ptr = 1 + strchr(hdr_ptr, '\0');
+
+    /* Skip all null characters with which the line terminators were overwritten */
+    while (*hdr_ptr == '\0') {
+        hdr_ptr++;
+    }
+    return hdr_ptr;
+}
+
+/* Locate the value of a header field within the request's scratch buffer.
+ * Returns a pointer to the NULL-terminated value (leading spaces skipped),
+ * or NULL if the field is not present. The caller must validate r/field and
+ * the request pointer before calling. */
+static const char *httpd_find_hdr_value(struct httpd_req_aux *ra, const char *field)
+{
+    const char  *hdr_ptr   = ra->scratch;        /*!< Request headers are kept in scratch buffer */
+    unsigned     count     = ra->req_hdrs_count; /*!< Count set during parsing  */
+    const size_t field_len = strlen(field);
+
+    while (count--) {
+        /* Search for the ':' character. Else, it would mean
+         * that the field is invalid */
+        const char *val_ptr = strchr(hdr_ptr, ':');
+        if (!val_ptr) {
+            break;
+        }
+
+        /* If the field does not match, continue searching. Compare lengths
+         * first as the field from the header is not null terminated (has ':'
+         * in the end). */
+        if (((size_t)(val_ptr - hdr_ptr) != field_len) ||
+            strncasecmp(hdr_ptr, field, field_len)) {
+            if (count) {
+                hdr_ptr = httpd_next_hdr(hdr_ptr);
+            }
+            continue;
+        }
+
+        /* Skip ':' and any preceding spaces */
+        val_ptr++;
+        while (*val_ptr == ' ') {
+            val_ptr++;
+        }
+        return val_ptr;
+    }
+    return NULL;
+}
+
 /* Get the length of the value string of a header request field */
 size_t httpd_req_get_hdr_value_len(httpd_req_t *r, const char *field)
 {
@@ -1047,48 +1133,8 @@ size_t httpd_req_get_hdr_value_len(httpd_req_t *r, const char *field)
         return 0;
     }
 
-    struct httpd_req_aux *ra = r->aux;
+    const char *val_ptr = httpd_find_hdr_value(r->aux, field);
-    const char   *hdr_ptr = ra->scratch;         /*!< Request headers are kept in scratch buffer */
+    return val_ptr ? strlen(val_ptr) : 0;
-    unsigned      count   = ra->req_hdrs_count;  /*!< Count set during parsing  */
-
-    while (count--) {
-        /* Search for the ':' character. Else, it would mean
-         * that the field is invalid
-         */
-        const char *val_ptr = strchr(hdr_ptr, ':');
-        if (!val_ptr) {
-            break;
-        }
-
-        /* If the field, does not match, continue searching.
-         * Compare lengths first as field from header is not
-         * null terminated (has ':' in the end).
-         */
-        if ((val_ptr - hdr_ptr != strlen(field)) ||
-            (strncasecmp(hdr_ptr, field, strlen(field)))) {
-            if (count) {
-                /* Jump to end of header field-value string */
-                hdr_ptr = 1 + strchr(hdr_ptr, '\0');
-
-                /* Skip all null characters (with which the line
-                 * terminators had been overwritten) */
-                while (*hdr_ptr == '\0') {
-                    hdr_ptr++;
-                }
-            }
-            continue;
-        }
-
-        /* Skip ':' */
-        val_ptr++;
-
-        /* Skip preceding space */
-        while ((*val_ptr != '\0') && (*val_ptr == ' ')) {
-            val_ptr++;
-        }
-        return strlen(val_ptr);
-    }
-    return 0;
 }
 
 /* Get the value of a field from the request headers */
@@ -1102,58 +1148,46 @@ esp_err_t httpd_req_get_hdr_value_str(httpd_req_t *r, const char *field, char *v
         return ESP_ERR_HTTPD_INVALID_REQ;
     }
 
-    struct httpd_req_aux *ra = r->aux;
+    const char *val_ptr = httpd_find_hdr_value(r->aux, field);
-    const char   *hdr_ptr = ra->scratch;         /*!< Request headers are kept in scratch buffer */
+    if (val_ptr == NULL) {
-    unsigned     count    = ra->req_hdrs_count;  /*!< Count set during parsing  */
+        return ESP_ERR_NOT_FOUND;
-
-    while (count--) {
-        /* Search for the ':' character. Else, it would mean
-         * that the field is invalid
-         */
-        const char *val_ptr = strchr(hdr_ptr, ':');
-        if (!val_ptr) {
-            break;
-        }
-
-        /* If the field, does not match, continue searching.
-         * Compare lengths first as field from header is not
-         * null terminated (has ':' in the end).
-         */
-        if ((val_ptr - hdr_ptr != strlen(field)) ||
-            (strncasecmp(hdr_ptr, field, strlen(field)))) {
-            if (count) {
-                /* Jump to end of header field-value string */
-                hdr_ptr = 1 + strchr(hdr_ptr, '\0');
-
-                /* Skip all null characters (with which the line
-                 * terminators had been overwritten) */
-                while (*hdr_ptr == '\0') {
-                    hdr_ptr++;
-                }
-            }
-            continue;
-        }
-
-        /* Skip ':' */
-        val_ptr++;
-
-        /* Skip preceding space */
-        while ((*val_ptr != '\0') && (*val_ptr == ' ')) {
-            val_ptr++;
-        }
-
-        /* Get the NULL terminated value and copy it to the caller's buffer.
-         * Note `strlcpy()` will always return the size of the source string
-         * including terminimating null.*/
-        size_t full_size = strlcpy(val, val_ptr, val_size);
-
-        /* If buffer length is smaller than needed, return truncation error */
-        if (val_size < full_size) {
-            return ESP_ERR_HTTPD_RESULT_TRUNC;
-        }
-        return ESP_OK;
     }
-    return ESP_ERR_NOT_FOUND;
+
+    /* Get the NULL terminated value and copy it to the caller's buffer.
+     * strlcpy() returns strlen() of the source string (the terminating
+     * null is NOT counted), so only val_size - 1 characters fit. */
+    size_t full_size = strlcpy(val, val_ptr, val_size);
+
+    /* If the value did not fit in the buffer it was truncated, i.e. its
+     * length reached or exceeded val_size. Return truncation error. */
+    if (val_size <= full_size) {
+        return ESP_ERR_HTTPD_RESULT_TRUNC;
+    }
+    return ESP_OK;
+}
+
+esp_err_t httpd_req_get_hdr_value_str_ptr(httpd_req_t *r, const char *field, const char **val, size_t *val_len)
+{
+    /* val and val_len are output pointers that are dereferenced below, so they
+     * must be validated along with the other arguments to avoid a NULL write */
+    if (r == NULL || field == NULL || val == NULL || val_len == NULL) {
+        return ESP_ERR_INVALID_ARG;
+    }
+
+    if (!httpd_valid_req(r)) {
+        return ESP_ERR_HTTPD_INVALID_REQ;
+    }
+
+    const char *val_ptr = httpd_find_hdr_value(r->aux, field);
+    if (val_ptr == NULL) {
+        return ESP_ERR_NOT_FOUND;
+    }
+
+    /* The value lives NULL-terminated in the request scratch buffer; return a
+     * pointer to it instead of copying. Valid only within the URI handler. */
+    *val     = val_ptr;
+    *val_len = strlen(val_ptr);
+    return ESP_OK;
 }
 
 /* Helper function to get a cookie value from a cookie string of the type "cookie1=val1; cookie2=val2" */

components/esp_http_server/test_apps/main/test_http_server.c

@@ -15,9 +15,10 @@
 #include "freertos/FreeRTOS.h"
 #include "freertos/semphr.h"
 
-#ifdef CONFIG_HTTPD_WS_SUPPORT
+/* http_parser.h must precede esp_httpd_priv.h, which embeds a
+ * struct http_parser_url by value */
+#include <http_parser.h>
 #include "../../src/esp_httpd_priv.h"
-#endif
 
 #include "unity.h"
 #include "test_utils.h"
@@ -670,6 +671,141 @@ TEST_CASE("WS handshake invalid Sec-WebSocket-Key returns 400", "[HTTP SERVER][w
 
 #endif /* CONFIG_HTTPD_WS_SUPPORT */
 
+/********* URL query / header pointer-accessor tests *********
+ * These exercise httpd_req_get_url_query_str_ptr() and
+ * httpd_req_get_hdr_value_str_ptr() against hand-built requests that mirror
+ * the parser's output: the query is parsed with http_parser_parse_url() (the
+ * exact mechanism the server uses), and the header scratch buffer reproduces
+ * the parser layout ("Field: value" with the CRLF terminators replaced by
+ * null bytes). The new pointer APIs are cross-checked against the existing
+ * copy/length variants so any divergence is caught.
+ *
+ * Note: these assume CONFIG_HTTPD_VALIDATE_REQ is disabled (the default), so
+ * httpd_valid_req() accepts the stack request used here. */
+
+/* Parse a query-carrying URI into a stack request, like verify_url() does */
+static void build_query_req(httpd_req_t *req, struct httpd_req_aux *aux, const char *uri)
+{
+    memset(req, 0, sizeof(*req));
+    memset(aux, 0, sizeof(*aux));
+    req->aux = aux;
+    strlcpy((char *)req->uri, uri, sizeof(req->uri));
+    http_parser_url_init(&aux->url_parse_res);
+    TEST_ASSERT_EQUAL(0, http_parser_parse_url(req->uri, strlen(req->uri), 0,
+                                               &aux->url_parse_res));
+}
+
+TEST_CASE("httpd_req_get_url_query_str_ptr returns query without copy", "[HTTP SERVER]")
+{
+    httpd_req_t req;
+    struct httpd_req_aux aux;
+    const char *expected = "foo=bar&baz=qux";
+    build_query_req(&req, &aux, "/path?foo=bar&baz=qux");
+
+    const char *q = NULL;
+    size_t qlen = 0;
+    TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
+    TEST_ASSERT_NOT_NULL(q);
+    TEST_ASSERT_EQUAL(strlen(expected), qlen);
+    TEST_ASSERT_EQUAL(0, memcmp(q, expected, qlen));
+
+    /* Must point into the request URI buffer, i.e. no copy was made */
+    TEST_ASSERT_TRUE(q >= req.uri && q < req.uri + sizeof(req.uri));
+
+    /* Cross-check against the length and copy variants */
+    TEST_ASSERT_EQUAL(qlen, httpd_req_get_url_query_len(&req));
+    char buf[64];
+    TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_url_query_str(&req, buf, sizeof(buf)));
+    TEST_ASSERT_EQUAL(0, strncmp(buf, q, qlen));
+}
+
+TEST_CASE("httpd_req_get_url_query_str_ptr handles empty and missing query", "[HTTP SERVER]")
+{
+    httpd_req_t req = {0};
+    struct httpd_req_aux aux = {0};
+    req.aux = &aux;
+    const char *q = NULL;
+    size_t qlen = 0;
+
+    /* Empty URI -> ESP_FAIL */
+    TEST_ASSERT_EQUAL(ESP_FAIL, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
+
+    /* URI without a query -> ESP_ERR_NOT_FOUND */
+    build_query_req(&req, &aux, "/path/only");
+    TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
+}
+
+TEST_CASE("httpd_req_get_url_query_str_ptr validates NULL args", "[HTTP SERVER][security]")
+{
+    httpd_req_t req;
+    struct httpd_req_aux aux;
+    build_query_req(&req, &aux, "/p?x=1");
+
+    const char *q = NULL;
+    size_t qlen = 0;
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(NULL, &q, &qlen));
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(&req, NULL, &qlen));
+    /* Regression: a query is present, so a NULL buf_len used to be written to */
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(&req, &q, NULL));
+}
+
+TEST_CASE("httpd_req_get_hdr_value_str_ptr returns value without copy", "[HTTP SERVER]")
+{
+    httpd_req_t req = {0};
+    struct httpd_req_aux aux = {0};
+    /* Parser layout: "Field: value" entries, CRLF terminators replaced by nulls */
+    static char scratch[] = "Host: example.com\0\0X-Custom: hello world";
+    aux.scratch = scratch;
+    aux.req_hdrs_count = 2;
+    req.aux = &aux;
+
+    const char *val = NULL;
+    size_t vlen = 0;
+    TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", &val, &vlen));
+    TEST_ASSERT_EQUAL(strlen("hello world"), vlen);
+    TEST_ASSERT_EQUAL(0, strcmp(val, "hello world"));
+
+    /* Must point into the scratch buffer, i.e. no copy was made */
+    TEST_ASSERT_TRUE(val >= scratch && val < scratch + sizeof(scratch));
+
+    /* Field match is case-insensitive, like the copy variant */
+    TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str_ptr(&req, "host", &val, &vlen));
+    TEST_ASSERT_EQUAL(0, strcmp(val, "example.com"));
+
+    /* Cross-check the new pointer API against the length and copy variants */
+    TEST_ASSERT_EQUAL(strlen("hello world"), httpd_req_get_hdr_value_len(&req, "X-Custom"));
+    char buf[32];
+    TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str(&req, "X-Custom", buf, sizeof(buf)));
+    TEST_ASSERT_EQUAL(0, strcmp(buf, "hello world"));
+
+    /* Missing field -> ESP_ERR_NOT_FOUND */
+    TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND,
+                      httpd_req_get_hdr_value_str_ptr(&req, "Nonexistent", &val, &vlen));
+}
+
+TEST_CASE("httpd_req_get_hdr_value_str_ptr validates NULL args", "[HTTP SERVER][security]")
+{
+    httpd_req_t req = {0};
+    struct httpd_req_aux aux = {0};
+    static char scratch[] = "X-Custom: hello world";
+    aux.scratch = scratch;
+    aux.req_hdrs_count = 1;
+    req.aux = &aux;
+
+    const char *val = NULL;
+    size_t vlen = 0;
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
+                      httpd_req_get_hdr_value_str_ptr(NULL, "X-Custom", &val, &vlen));
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
+                      httpd_req_get_hdr_value_str_ptr(&req, NULL, &val, &vlen));
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
+                      httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", NULL, &vlen));
+    /* Regression-style guard: a value is present, so a NULL val_len must be
+     * rejected rather than written to */
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
+                      httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", &val, NULL));
+}
+
 void app_main(void)
 {
     unity_run_menu();