From 6b0290682277164a6db296efe5d7a9562966330a Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 17 Jul 2025 11:17:06 +0800 Subject: [PATCH] fix(security): improve memory allocation handling in multiple components --- components/esp_driver_uart/src/uart_vfs.c | 9 ++++-- .../src/usb_serial_jtag_vfs.c | 7 +++- components/esp_hid/src/nimble_hidh.c | 18 ++++++++--- components/esp_http_client/esp_http_client.c | 7 ++-- components/esp_http_client/lib/http_utils.c | 15 ++++++--- components/esp_http_server/src/httpd_parse.c | 7 ++-- components/esp_netif/lwip/esp_netif_br_glue.c | 14 +++++--- .../esp_netif/vfs_l2tap/esp_vfs_l2tap.c | 32 +++++++++++++++---- components/esp_vfs_console/vfs_cdcacm.c | 8 ++++- components/heap/heap_caps_linux.c | 12 ++++--- .../main/i2c_slave_main.c | 16 ++++++---- 11 files changed, 107 insertions(+), 38 deletions(-) diff --git a/components/esp_driver_uart/src/uart_vfs.c b/components/esp_driver_uart/src/uart_vfs.c index 9d547db385..332540ef85 100644 --- a/components/esp_driver_uart/src/uart_vfs.c +++ b/components/esp_driver_uart/src/uart_vfs.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -446,7 +446,12 @@ static esp_err_t unregister_select(uart_select_args_t *args) // The item is removed by overwriting it with the last item. The subsequent rellocation will drop the // last item. s_registered_selects[i] = s_registered_selects[new_size]; - s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(uart_select_args_t *), UART_VFS_MALLOC_FLAGS); + uart_select_args_t **new_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(uart_select_args_t *), UART_VFS_MALLOC_FLAGS); + if (new_selects == NULL && new_size > 0) { + ret = ESP_ERR_NO_MEM; + } else { + s_registered_selects = new_selects; + } // Shrinking a buffer with realloc is guaranteed to succeed. s_registered_select_num = new_size; ret = ESP_OK; diff --git a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c index c1d9faa340..e28c825704 100644 --- a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c +++ b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c @@ -432,7 +432,12 @@ static esp_err_t unregister_select(usb_serial_jtag_select_args_t *args) // The item is removed by overwriting it with the last item. The subsequent rellocation will drop the // last item. s_registered_selects[i] = s_registered_selects[new_size]; - s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(usb_serial_jtag_select_args_t *), USJ_VFS_MALLOC_FLAGS); + usb_serial_jtag_select_args_t **new_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(usb_serial_jtag_select_args_t *), USJ_VFS_MALLOC_FLAGS); + if (new_selects == NULL && new_size > 0) { + ret = ESP_ERR_NO_MEM; + } else { + s_registered_selects = new_selects; + } // Shrinking a buffer with realloc is guaranteed to succeed. s_registered_select_num = new_size; diff --git a/components/esp_hid/src/nimble_hidh.c b/components/esp_hid/src/nimble_hidh.c index 134baca889..22b08cbc37 100644 --- a/components/esp_hid/src/nimble_hidh.c +++ b/components/esp_hid/src/nimble_hidh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -106,12 +106,20 @@ nimble_on_read(uint16_t conn_handle, MODLOG_DFLT(DEBUG, " attr_handle=%d value=", attr->handle); old_offset = s_read_data_len; s_read_data_len += OS_MBUF_PKTLEN(attr->om); - s_read_data_val = realloc(s_read_data_val, s_read_data_len + 1); // 1 extra byte to store null char + uint8_t *tmp = realloc(s_read_data_val, s_read_data_len + 1); + if (tmp == NULL) { + ESP_LOGE(TAG, "Failed to allocate memory for read data"); + free(s_read_data_val); + s_read_data_val = NULL; + s_read_data_len = 0; + return -1; + } + s_read_data_val = tmp; ble_hs_mbuf_to_flat(attr->om, s_read_data_val + old_offset, OS_MBUF_PKTLEN(attr->om), NULL); print_mbuf(attr->om); return 0; case BLE_HS_EDONE: - s_read_data_val[s_read_data_len] = 0; // to insure strings are ended with \0 */ + s_read_data_val[s_read_data_len] = 0; // to ensure strings are ended with \0 */ s_read_status = 0; SEND_CB(); return 0; @@ -289,7 +297,7 @@ desc_disced(uint16_t conn_handle, const struct ble_gatt_error *error, } /* this api does the following things : -** does service, characteristic and discriptor discovery and +** does service, characteristic and descriptor discovery and ** fills the hid device information accordingly in dev */ static void read_device_services(esp_hidh_dev_t *dev) { @@ -466,7 +474,7 @@ static void read_device_services(esp_hidh_dev_t *dev) chr_end_handle, desc_disced, descr_result); WAIT_CB(); if (status != 0) { - ESP_LOGE(TAG, "failed to find discriptors for characteristic : %d", c); + ESP_LOGE(TAG, "failed to find descriptors for characteristic : %d", c); assert(status == 0); } dcount = dscs_discovered; diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 4c04846549..ad07ade5ab 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -349,11 +349,14 @@ static int http_on_body(http_parser *parser, const char *at, size_t length) ESP_LOGD(TAG, "Body received in fetch header state, %p, %zu", at, length); esp_http_buffer_t *res_buffer = client->response->buffer; assert(res_buffer->orig_raw_data == res_buffer->raw_data); - res_buffer->orig_raw_data = (char *)realloc(res_buffer->orig_raw_data, res_buffer->raw_len + length); - if (!res_buffer->orig_raw_data) { + char *tmp = (char *)realloc(res_buffer->orig_raw_data, res_buffer->raw_len + length); + if (tmp == NULL) { ESP_LOGE(TAG, "Failed to allocate memory for storing decoded data"); + free(res_buffer->orig_raw_data); + res_buffer->orig_raw_data = NULL; return -1; } + res_buffer->orig_raw_data = tmp; memcpy(res_buffer->orig_raw_data + res_buffer->raw_len, at, length); res_buffer->raw_data = res_buffer->orig_raw_data; } diff --git a/components/esp_http_client/lib/http_utils.c b/components/esp_http_client/lib/http_utils.c index 07e9c5585f..20f7a397c7 100644 --- a/components/esp_http_client/lib/http_utils.c +++ b/components/esp_http_client/lib/http_utils.c @@ -45,9 +45,16 @@ char *http_utils_assign_string(char **str, const char *new_str, int len) l = strlen(new_str); } if (old_str) { - old_str = realloc(old_str, l + 1); - ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted"); - old_str[l] = 0; + // old_str should not be reallocated directly, as in case of memory exhaustion, + // it will be lost and we will not be able to free it. + char *tmp = realloc(old_str, l + 1); + if (tmp == NULL) { + free(old_str); + old_str = NULL; + ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted"); + } + old_str = tmp; + old_str[l] = 0; // Ensure the new string is null-terminated } else { old_str = calloc(1, l + 1); ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted"); @@ -74,7 +81,7 @@ char *http_utils_append_string(char **str, const char *new_str, int len) if (tmp == NULL) { free(old_str); old_str = NULL; - ESP_RETURN_ON_FALSE(tmp, NULL, TAG, "Memory exhausted"); + ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted"); } old_str = tmp; // Ensure the new string is null-terminated diff --git a/components/esp_http_server/src/httpd_parse.c b/components/esp_http_server/src/httpd_parse.c index 357480790c..176a21d885 100644 --- a/components/esp_http_server/src/httpd_parse.c +++ b/components/esp_http_server/src/httpd_parse.c @@ -505,11 +505,14 @@ static int read_block(httpd_req_t *req, http_parser *parser, size_t offset, size from where the reading will start and buf_len is till what length the buffer will be read. */ - raux->scratch = (char*) realloc(raux->scratch, offset + buf_len); - if (raux->scratch == NULL) { + char *new_scratch = (char *) realloc(raux->scratch, offset + buf_len); + if (new_scratch == NULL) { + free(raux->scratch); + raux->scratch = NULL; ESP_LOGE(TAG, "Unable to allocate the scratch buffer"); return 0; } + raux->scratch = new_scratch; parser_data->last.at = raux->scratch + at_offset; raux->scratch_cur_size = offset + buf_len; ESP_LOGD(TAG, "scratch buf qsize = %d", raux->scratch_cur_size); diff --git a/components/esp_netif/lwip/esp_netif_br_glue.c b/components/esp_netif/lwip/esp_netif_br_glue.c index 4b31af6707..6a986a0b0e 100644 --- a/components/esp_netif/lwip/esp_netif_br_glue.c +++ b/components/esp_netif/lwip/esp_netif_br_glue.c @@ -329,11 +329,15 @@ esp_err_t esp_netif_br_glue_add_port(esp_netif_br_glue_handle_t netif_br_glue, e if (netif_br_glue->ports_esp_netifs == NULL) { netif_br_glue->ports_esp_netifs = malloc(sizeof(esp_netif_t *)); } else { - netif_br_glue->ports_esp_netifs = realloc(netif_br_glue->ports_esp_netifs, (netif_br_glue->port_cnt + 1) * sizeof(esp_netif_t *)); - } - if (!netif_br_glue->ports_esp_netifs) { - ESP_LOGE(TAG, "no memory to add br port"); - return ESP_ERR_NO_MEM; + esp_netif_t **new_ports = realloc(netif_br_glue->ports_esp_netifs, (netif_br_glue->port_cnt + 1) * sizeof(esp_netif_t *)); + if (new_ports == NULL) { + free(netif_br_glue->ports_esp_netifs); + netif_br_glue->ports_esp_netifs = NULL; + netif_br_glue->port_cnt = 0; + ESP_LOGE(TAG, "no memory to add br port"); + return ESP_ERR_NO_MEM; + } + netif_br_glue->ports_esp_netifs = new_ports; } netif_br_glue->ports_esp_netifs[netif_br_glue->port_cnt] = esp_netif_port; diff --git a/components/esp_netif/vfs_l2tap/esp_vfs_l2tap.c b/components/esp_netif/vfs_l2tap/esp_vfs_l2tap.c index 0b338b518d..399d99dc69 100644 --- a/components/esp_netif/vfs_l2tap/esp_vfs_l2tap.c +++ b/components/esp_netif/vfs_l2tap/esp_vfs_l2tap.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -641,6 +641,7 @@ static esp_err_t register_select(l2tap_select_args_t *args) const int new_size = s_registered_select_cnt + 1; l2tap_select_args_t **registered_selects_new; if ((registered_selects_new = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *))) == NULL) { + s_registered_selects = NULL; ret = ESP_ERR_NO_MEM; } else { s_registered_selects = registered_selects_new; @@ -663,13 +664,32 @@ static esp_err_t unregister_select(l2tap_select_args_t *args) const int new_size = s_registered_select_cnt - 1; // The item is removed by overwriting it with the last item. The subsequent rellocation will drop the // last item. - s_registered_selects[i] = s_registered_selects[new_size]; - s_registered_selects = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *)); - if (s_registered_selects || new_size == 0) { - s_registered_select_cnt = new_size; + // Move last element to fill gap (only if not removing the last element) + if (i < new_size) { + s_registered_selects[i] = s_registered_selects[new_size]; + } + // Handle reallocation + if (new_size == 0) { + // Free the entire array + free(s_registered_selects); + s_registered_selects = NULL; + s_registered_select_cnt = 0; ret = ESP_OK; } else { - ret = ESP_ERR_NO_MEM; + // Shrink the array + l2tap_select_args_t **new_selects = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *)); + if (new_selects == NULL) { + // Realloc failed - restore the moved element and return error + if (i < new_size) { + s_registered_selects[new_size] = s_registered_selects[i]; + } + ret = ESP_ERR_NO_MEM; + } else { + // Success - update pointer and count atomically + s_registered_selects = new_selects; + s_registered_select_cnt = new_size; + ret = ESP_OK; + } } break; } diff --git a/components/esp_vfs_console/vfs_cdcacm.c b/components/esp_vfs_console/vfs_cdcacm.c index 2575909cd4..8b4331116c 100644 --- a/components/esp_vfs_console/vfs_cdcacm.c +++ b/components/esp_vfs_console/vfs_cdcacm.c @@ -385,7 +385,13 @@ static esp_err_t unregister_select(cdcacm_select_args_t *args) // The item is removed by overwriting it with the last item. The subsequent rellocation will drop the // last item. s_registered_selects[i] = s_registered_selects[new_size]; - s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(cdcacm_select_args_t *), CDCACM_VFS_MALLOC_FLAGS); + cdcacm_select_args_t **new_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(cdcacm_select_args_t *), CDCACM_VFS_MALLOC_FLAGS); + if (new_selects == NULL && new_size > 0) { + ret = ESP_ERR_NO_MEM; + break; + } else { + s_registered_selects = new_selects; + } // Shrinking a buffer with realloc is guaranteed to succeed. s_registered_select_num = new_size; diff --git a/components/heap/heap_caps_linux.c b/components/heap/heap_caps_linux.c index c7b558e766..c462f42c5e 100644 --- a/components/heap/heap_caps_linux.c +++ b/components/heap/heap_caps_linux.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -84,11 +84,15 @@ void *heap_caps_malloc_prefer( size_t size, size_t num, ... ) static void *heap_caps_realloc_base( void *ptr, size_t size, uint32_t caps) { - ptr = realloc(ptr, size); - - if (ptr == NULL && size > 0) { + void *new_ptr = realloc(ptr, size); + if (new_ptr == NULL && size > 0) { heap_caps_alloc_failed(size, caps, __func__); } + // If realloc fails, it returns NULL and the original pointer is left unchanged. + // If realloc succeeds, it returns a pointer to the allocated memory, which may be the + // same as the original pointer or a new pointer if the memory was moved. + // We return the new pointer, which may be the same as the original pointer. + ptr = new_ptr; return ptr; } diff --git a/examples/peripherals/i2c/i2c_slave_network_sensor/main/i2c_slave_main.c b/examples/peripherals/i2c/i2c_slave_network_sensor/main/i2c_slave_main.c index e7c19e5b7a..d5d4a98a12 100644 --- a/examples/peripherals/i2c/i2c_slave_network_sensor/main/i2c_slave_main.c +++ b/examples/peripherals/i2c/i2c_slave_network_sensor/main/i2c_slave_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -62,11 +62,15 @@ static esp_err_t _http_event_handler(esp_http_client_event_t *evt) if (context->json_buffer == NULL) { context->json_buffer = malloc(evt->data_len + 1); } else { - context->json_buffer = realloc(context->json_buffer, context->json_size + evt->data_len + 1); - } - if (context->json_buffer == NULL) { - ESP_LOGE("HTTP_CLIENT", "Failed to allocate memory for data json_buffer"); - return ESP_FAIL; + char *new_buffer = realloc(context->json_buffer, context->json_size + evt->data_len + 1); + if (new_buffer == NULL) { + free(context->json_buffer); + context->json_buffer = NULL; + context->json_size = 0; + ESP_LOGE("HTTP_CLIENT", "Failed to allocate memory for data json_buffer"); + return ESP_FAIL; + } + context->json_buffer = new_buffer; } memcpy(context->json_buffer + context->json_size, evt->data, evt->data_len); context->json_size += evt->data_len;