Skip to content

Commit 65cf3d0

Browse files
committed
Merge branch 'bugfix/fix_dangerous_realloc_patterns' into 'master'
fix(security): improve memory allocation handling in multiple components Closes IDF-13612 and IDF-13622 See merge request espressif/esp-idf!40629
2 parents de58c6d + 6b02906 commit 65cf3d0

File tree

11 files changed

+107
-38
lines changed

11 files changed

+107
-38
lines changed

components/esp_driver_uart/src/uart_vfs.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -446,7 +446,12 @@ static esp_err_t unregister_select(uart_select_args_t *args)
446446
// The item is removed by overwriting it with the last item. The subsequent rellocation will drop the
447447
// last item.
448448
s_registered_selects[i] = s_registered_selects[new_size];
449-
s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(uart_select_args_t *), UART_VFS_MALLOC_FLAGS);
449+
uart_select_args_t **new_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(uart_select_args_t *), UART_VFS_MALLOC_FLAGS);
450+
if (new_selects == NULL && new_size > 0) {
451+
ret = ESP_ERR_NO_MEM;
452+
} else {
453+
s_registered_selects = new_selects;
454+
}
450455
// Shrinking a buffer with realloc is guaranteed to succeed.
451456
s_registered_select_num = new_size;
452457
ret = ESP_OK;

components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,12 @@ static esp_err_t unregister_select(usb_serial_jtag_select_args_t *args)
432432
// The item is removed by overwriting it with the last item. The subsequent rellocation will drop the
433433
// last item.
434434
s_registered_selects[i] = s_registered_selects[new_size];
435-
s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(usb_serial_jtag_select_args_t *), USJ_VFS_MALLOC_FLAGS);
435+
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);
436+
if (new_selects == NULL && new_size > 0) {
437+
ret = ESP_ERR_NO_MEM;
438+
} else {
439+
s_registered_selects = new_selects;
440+
}
436441
// Shrinking a buffer with realloc is guaranteed to succeed.
437442
s_registered_select_num = new_size;
438443

components/esp_hid/src/nimble_hidh.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2017-2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2017-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -106,12 +106,20 @@ nimble_on_read(uint16_t conn_handle,
106106
MODLOG_DFLT(DEBUG, " attr_handle=%d value=", attr->handle);
107107
old_offset = s_read_data_len;
108108
s_read_data_len += OS_MBUF_PKTLEN(attr->om);
109-
s_read_data_val = realloc(s_read_data_val, s_read_data_len + 1); // 1 extra byte to store null char
109+
uint8_t *tmp = realloc(s_read_data_val, s_read_data_len + 1);
110+
if (tmp == NULL) {
111+
ESP_LOGE(TAG, "Failed to allocate memory for read data");
112+
free(s_read_data_val);
113+
s_read_data_val = NULL;
114+
s_read_data_len = 0;
115+
return -1;
116+
}
117+
s_read_data_val = tmp;
110118
ble_hs_mbuf_to_flat(attr->om, s_read_data_val + old_offset, OS_MBUF_PKTLEN(attr->om), NULL);
111119
print_mbuf(attr->om);
112120
return 0;
113121
case BLE_HS_EDONE:
114-
s_read_data_val[s_read_data_len] = 0; // to insure strings are ended with \0 */
122+
s_read_data_val[s_read_data_len] = 0; // to ensure strings are ended with \0 */
115123
s_read_status = 0;
116124
SEND_CB();
117125
return 0;
@@ -289,7 +297,7 @@ desc_disced(uint16_t conn_handle, const struct ble_gatt_error *error,
289297
}
290298

291299
/* this api does the following things :
292-
** does service, characteristic and discriptor discovery and
300+
** does service, characteristic and descriptor discovery and
293301
** fills the hid device information accordingly in dev */
294302
static void read_device_services(esp_hidh_dev_t *dev)
295303
{
@@ -466,7 +474,7 @@ static void read_device_services(esp_hidh_dev_t *dev)
466474
chr_end_handle, desc_disced, descr_result);
467475
WAIT_CB();
468476
if (status != 0) {
469-
ESP_LOGE(TAG, "failed to find discriptors for characteristic : %d", c);
477+
ESP_LOGE(TAG, "failed to find descriptors for characteristic : %d", c);
470478
assert(status == 0);
471479
}
472480
dcount = dscs_discovered;

components/esp_http_client/esp_http_client.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,14 @@ static int http_on_body(http_parser *parser, const char *at, size_t length)
349349
ESP_LOGD(TAG, "Body received in fetch header state, %p, %zu", at, length);
350350
esp_http_buffer_t *res_buffer = client->response->buffer;
351351
assert(res_buffer->orig_raw_data == res_buffer->raw_data);
352-
res_buffer->orig_raw_data = (char *)realloc(res_buffer->orig_raw_data, res_buffer->raw_len + length);
353-
if (!res_buffer->orig_raw_data) {
352+
char *tmp = (char *)realloc(res_buffer->orig_raw_data, res_buffer->raw_len + length);
353+
if (tmp == NULL) {
354354
ESP_LOGE(TAG, "Failed to allocate memory for storing decoded data");
355+
free(res_buffer->orig_raw_data);
356+
res_buffer->orig_raw_data = NULL;
355357
return -1;
356358
}
359+
res_buffer->orig_raw_data = tmp;
357360
memcpy(res_buffer->orig_raw_data + res_buffer->raw_len, at, length);
358361
res_buffer->raw_data = res_buffer->orig_raw_data;
359362
}

components/esp_http_client/lib/http_utils.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,16 @@ char *http_utils_assign_string(char **str, const char *new_str, int len)
4545
l = strlen(new_str);
4646
}
4747
if (old_str) {
48-
old_str = realloc(old_str, l + 1);
49-
ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted");
50-
old_str[l] = 0;
48+
// old_str should not be reallocated directly, as in case of memory exhaustion,
49+
// it will be lost and we will not be able to free it.
50+
char *tmp = realloc(old_str, l + 1);
51+
if (tmp == NULL) {
52+
free(old_str);
53+
old_str = NULL;
54+
ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted");
55+
}
56+
old_str = tmp;
57+
old_str[l] = 0; // Ensure the new string is null-terminated
5158
} else {
5259
old_str = calloc(1, l + 1);
5360
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)
7481
if (tmp == NULL) {
7582
free(old_str);
7683
old_str = NULL;
77-
ESP_RETURN_ON_FALSE(tmp, NULL, TAG, "Memory exhausted");
84+
ESP_RETURN_ON_FALSE(old_str, NULL, TAG, "Memory exhausted");
7885
}
7986
old_str = tmp;
8087
// Ensure the new string is null-terminated

components/esp_http_server/src/httpd_parse.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,11 +505,14 @@ static int read_block(httpd_req_t *req, http_parser *parser, size_t offset, size
505505
from where the reading will start and buf_len is till what length
506506
the buffer will be read.
507507
*/
508-
raux->scratch = (char*) realloc(raux->scratch, offset + buf_len);
509-
if (raux->scratch == NULL) {
508+
char *new_scratch = (char *) realloc(raux->scratch, offset + buf_len);
509+
if (new_scratch == NULL) {
510+
free(raux->scratch);
511+
raux->scratch = NULL;
510512
ESP_LOGE(TAG, "Unable to allocate the scratch buffer");
511513
return 0;
512514
}
515+
raux->scratch = new_scratch;
513516
parser_data->last.at = raux->scratch + at_offset;
514517
raux->scratch_cur_size = offset + buf_len;
515518
ESP_LOGD(TAG, "scratch buf qsize = %d", raux->scratch_cur_size);

components/esp_netif/lwip/esp_netif_br_glue.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,11 +329,15 @@ esp_err_t esp_netif_br_glue_add_port(esp_netif_br_glue_handle_t netif_br_glue, e
329329
if (netif_br_glue->ports_esp_netifs == NULL) {
330330
netif_br_glue->ports_esp_netifs = malloc(sizeof(esp_netif_t *));
331331
} else {
332-
netif_br_glue->ports_esp_netifs = realloc(netif_br_glue->ports_esp_netifs, (netif_br_glue->port_cnt + 1) * sizeof(esp_netif_t *));
333-
}
334-
if (!netif_br_glue->ports_esp_netifs) {
335-
ESP_LOGE(TAG, "no memory to add br port");
336-
return ESP_ERR_NO_MEM;
332+
esp_netif_t **new_ports = realloc(netif_br_glue->ports_esp_netifs, (netif_br_glue->port_cnt + 1) * sizeof(esp_netif_t *));
333+
if (new_ports == NULL) {
334+
free(netif_br_glue->ports_esp_netifs);
335+
netif_br_glue->ports_esp_netifs = NULL;
336+
netif_br_glue->port_cnt = 0;
337+
ESP_LOGE(TAG, "no memory to add br port");
338+
return ESP_ERR_NO_MEM;
339+
}
340+
netif_br_glue->ports_esp_netifs = new_ports;
337341
}
338342

339343
netif_br_glue->ports_esp_netifs[netif_br_glue->port_cnt] = esp_netif_port;

components/esp_netif/vfs_l2tap/esp_vfs_l2tap.c

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -641,6 +641,7 @@ static esp_err_t register_select(l2tap_select_args_t *args)
641641
const int new_size = s_registered_select_cnt + 1;
642642
l2tap_select_args_t **registered_selects_new;
643643
if ((registered_selects_new = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *))) == NULL) {
644+
s_registered_selects = NULL;
644645
ret = ESP_ERR_NO_MEM;
645646
} else {
646647
s_registered_selects = registered_selects_new;
@@ -663,13 +664,32 @@ static esp_err_t unregister_select(l2tap_select_args_t *args)
663664
const int new_size = s_registered_select_cnt - 1;
664665
// The item is removed by overwriting it with the last item. The subsequent rellocation will drop the
665666
// last item.
666-
s_registered_selects[i] = s_registered_selects[new_size];
667-
s_registered_selects = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *));
668-
if (s_registered_selects || new_size == 0) {
669-
s_registered_select_cnt = new_size;
667+
// Move last element to fill gap (only if not removing the last element)
668+
if (i < new_size) {
669+
s_registered_selects[i] = s_registered_selects[new_size];
670+
}
671+
// Handle reallocation
672+
if (new_size == 0) {
673+
// Free the entire array
674+
free(s_registered_selects);
675+
s_registered_selects = NULL;
676+
s_registered_select_cnt = 0;
670677
ret = ESP_OK;
671678
} else {
672-
ret = ESP_ERR_NO_MEM;
679+
// Shrink the array
680+
l2tap_select_args_t **new_selects = realloc(s_registered_selects, new_size * sizeof(l2tap_select_args_t *));
681+
if (new_selects == NULL) {
682+
// Realloc failed - restore the moved element and return error
683+
if (i < new_size) {
684+
s_registered_selects[new_size] = s_registered_selects[i];
685+
}
686+
ret = ESP_ERR_NO_MEM;
687+
} else {
688+
// Success - update pointer and count atomically
689+
s_registered_selects = new_selects;
690+
s_registered_select_cnt = new_size;
691+
ret = ESP_OK;
692+
}
673693
}
674694
break;
675695
}

components/esp_vfs_console/vfs_cdcacm.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,13 @@ static esp_err_t unregister_select(cdcacm_select_args_t *args)
385385
// The item is removed by overwriting it with the last item. The subsequent rellocation will drop the
386386
// last item.
387387
s_registered_selects[i] = s_registered_selects[new_size];
388-
s_registered_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(cdcacm_select_args_t *), CDCACM_VFS_MALLOC_FLAGS);
388+
cdcacm_select_args_t **new_selects = heap_caps_realloc(s_registered_selects, new_size * sizeof(cdcacm_select_args_t *), CDCACM_VFS_MALLOC_FLAGS);
389+
if (new_selects == NULL && new_size > 0) {
390+
ret = ESP_ERR_NO_MEM;
391+
break;
392+
} else {
393+
s_registered_selects = new_selects;
394+
}
389395
// Shrinking a buffer with realloc is guaranteed to succeed.
390396
s_registered_select_num = new_size;
391397

components/heap/heap_caps_linux.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -84,11 +84,15 @@ void *heap_caps_malloc_prefer( size_t size, size_t num, ... )
8484

8585
static void *heap_caps_realloc_base( void *ptr, size_t size, uint32_t caps)
8686
{
87-
ptr = realloc(ptr, size);
88-
89-
if (ptr == NULL && size > 0) {
87+
void *new_ptr = realloc(ptr, size);
88+
if (new_ptr == NULL && size > 0) {
9089
heap_caps_alloc_failed(size, caps, __func__);
9190
}
91+
// If realloc fails, it returns NULL and the original pointer is left unchanged.
92+
// If realloc succeeds, it returns a pointer to the allocated memory, which may be the
93+
// same as the original pointer or a new pointer if the memory was moved.
94+
// We return the new pointer, which may be the same as the original pointer.
95+
ptr = new_ptr;
9296

9397
return ptr;
9498
}

0 commit comments

Comments
 (0)