From 7c4d3fbf8b47071eda6165640aac3a67419f1df5 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Mon, 2 Nov 2020 20:40:25 +0100 Subject: [PATCH 1/2] esp_netif: Fixed possible use of hostname pointer after its freed When setting hostname using esp_netif_set_hostname_api() failed for some reason, the netif pointer might be freed while lwip pointer stil point to that location and could be used. Fixed by moving the freeing and string duplication to the block where lwip hostname is set. Closes https://github.com/espressif/esp-idf/issues/6048 --- components/esp_netif/lwip/esp_netif_lwip.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/components/esp_netif/lwip/esp_netif_lwip.c b/components/esp_netif/lwip/esp_netif_lwip.c index 536bb597b8..8a3e2f16ff 100644 --- a/components/esp_netif/lwip/esp_netif_lwip.c +++ b/components/esp_netif/lwip/esp_netif_lwip.c @@ -1133,19 +1133,20 @@ static esp_err_t esp_netif_set_hostname_api(esp_netif_api_msg_t *msg) #if LWIP_NETIF_HOSTNAME struct netif *p_netif = esp_netif->lwip_netif; - if (esp_netif->hostname) { - free(esp_netif->hostname); - } - esp_netif->hostname = strdup(hostname); - if (esp_netif->hostname == NULL) { - return ESP_ERR_NO_MEM; - } if (strlen(hostname) > ESP_NETIF_HOSTNAME_MAX_SIZE) { return ESP_ERR_ESP_NETIF_INVALID_PARAMS; } if (p_netif != NULL) { + if (esp_netif->hostname) { + free(esp_netif->hostname); + } + esp_netif->hostname = strdup(hostname); + if (esp_netif->hostname == NULL) { + p_netif->hostname = CONFIG_LWIP_LOCAL_HOSTNAME; + return ESP_ERR_NO_MEM; + } p_netif->hostname = esp_netif->hostname; return ESP_OK; } else { From 916d2f6f4c64c359c569deac4e451dee0a24c9aa Mon Sep 17 00:00:00 2001 From: David Cermak Date: Mon, 2 Nov 2020 20:41:46 +0100 Subject: [PATCH 2/2] esp_netif: Added test for failing to set hostname --- components/esp_netif/test/test_esp_netif.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/esp_netif/test/test_esp_netif.c b/components/esp_netif/test/test_esp_netif.c index 69f158cd4f..5140be7bbe 100644 --- a/components/esp_netif/test/test_esp_netif.c +++ b/components/esp_netif/test/test_esp_netif.c @@ -275,5 +275,13 @@ TEST_CASE("esp_netif: get/set hostname", "[esp_netif]") TEST_ASSERT_EQUAL(ESP_OK, esp_netif_get_hostname(esp_netif, &hostname)); TEST_ASSERT_EQUAL_STRING(hostname, "new_name"); + // test that setting the long name is refused and the previously set value retained + #define ESP_NETIF_HOSTNAME_MAX_SIZE 32 + char long_name[ESP_NETIF_HOSTNAME_MAX_SIZE + 2] = { 0 }; + memset(long_name, 'A', ESP_NETIF_HOSTNAME_MAX_SIZE+1); // construct the long name + TEST_ASSERT_NOT_EQUAL(ESP_OK, esp_netif_set_hostname(esp_netif, long_name)); + TEST_ASSERT_EQUAL(ESP_OK, esp_netif_get_hostname(esp_netif, &hostname)); + TEST_ASSERT_EQUAL_STRING(hostname, "new_name"); + esp_netif_destroy(esp_netif); }