From a4b10f1691bd59ffddc3bb86da41463d574ec776 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 21 May 2026 15:57:30 +0800 Subject: [PATCH] fix(esp_http_client): clear URL credentials and digest auth state on host change --- components/esp_http_client/esp_http_client.c | 10 +++-- .../test_apps/main/test_http_client.c | 40 ++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 3cbe6fc1745..1647f26d6e7 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -1239,10 +1239,14 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u free(old_host); return ESP_ERR_NO_MEM; } - /* Cross-origin credential hygiene: an Authorization header set by the - * application for the original host must not be re-sent to a different - * host (e.g. an attacker-controlled redirect target). */ http_header_delete(client->request->headers, "Authorization"); + free(client->connection_info.username); + client->connection_info.username = NULL; + free(client->connection_info.password); + client->connection_info.password = NULL; + free(client->auth_header); + client->auth_header = NULL; + _clear_auth_data(client); /* Free cached data if any, as we are closing this connection */ esp_http_client_cached_buf_cleanup(client->response->buffer); esp_http_client_close(client); diff --git a/components/esp_http_client/test_apps/main/test_http_client.c b/components/esp_http_client/test_apps/main/test_http_client.c index ae43caf6799..31aafe2efaa 100644 --- a/components/esp_http_client/test_apps/main/test_http_client.c +++ b/components/esp_http_client/test_apps/main/test_http_client.c @@ -186,12 +186,6 @@ TEST_CASE("esp_http_client_set_header() should not return error if header value esp_http_client_cleanup(client); } -/** - * Cross-origin credential leak: an Authorization header set by the application - * must NOT be carried across a host change in esp_http_client_set_url (which - * happens on redirects). Failing to clear it leaks tokens to attacker-controlled - * hosts when the trusted server returns a 30x Location pointing elsewhere. - */ TEST_CASE("set_url() to a different host strips Authorization header", "[esp_http_client]") { esp_http_client_config_t config = { @@ -206,7 +200,7 @@ TEST_CASE("set_url() to a different host strips Authorization header", "[esp_htt TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_get_header(client, "Authorization", &value)); TEST_ASSERT_NOT_NULL(value); - /* Simulate an attacker-controlled redirect target */ + /* Simulate a redirect target */ TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_set_url(client, "http://attacker.example/steal")); value = NULL; @@ -217,8 +211,6 @@ TEST_CASE("set_url() to a different host strips Authorization header", "[esp_htt esp_http_client_cleanup(client); } -/* Regression guard: same-host set_url (e.g. redirect to a different path on the - * same origin) must preserve the Authorization header. */ TEST_CASE("set_url() to the same host preserves Authorization header", "[esp_http_client]") { esp_http_client_config_t config = { @@ -238,6 +230,36 @@ TEST_CASE("set_url() to the same host preserves Authorization header", "[esp_htt esp_http_client_cleanup(client); } +TEST_CASE("set_url() to a different host clears URL-embedded credentials", "[esp_http_client]") +{ + esp_http_client_config_t config = { + .host = HOST, + .path = "/", + .username = USERNAME, + .password = PASSWORD, + }; + esp_http_client_handle_t client = esp_http_client_init(&config); + TEST_ASSERT_NOT_NULL(client); + + char *value = NULL; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_get_username(client, &value)); + TEST_ASSERT_NOT_NULL(value); + value = NULL; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_get_password(client, &value)); + TEST_ASSERT_NOT_NULL(value); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_set_url(client, "http://attacker.example/steal")); + + value = NULL; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_get_username(client, &value)); + TEST_ASSERT_NULL(value); + value = NULL; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_get_password(client, &value)); + TEST_ASSERT_NULL(value); + + esp_http_client_cleanup(client); +} + static int disconnect_event_count = 0; static esp_err_t disconnect_event_handler(esp_http_client_event_t *evt)