From d554bbb98b127af168d452d709fec3b244f1f792 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 21 Aug 2026 00:13:03 +0200 Subject: [PATCH] F-7066: bound the FIT DTS relocation copy to the staging size The FIT boot path relocated the flat-dt sub-image with a copy whose length came from the FIT-declared data property length, never bounded against the WOLFBOOT_LOAD_DTS_ADDRESS staging region - unlike the sibling DTB paths, which all validate the parsed size against WOLFBOOT_DTS_MIN_SIZE/WOLFBOOT_DTS_MAX_SIZE first. The length was also harvested through a (int*)&dts_size cast of a uint32_t. Relocate the parsed DTB size instead: validate it against the same MIN/MAX bounds as the other DTB sources and copy that many bytes. An out-of-range DTB is rejected (dts_addr stays NULL and the existing fallback chain applies) rather than partially or oversize copied. Applied to both call sites of the pattern: update_ram.c (memcpy) and update_disk.c (wolfBoot_fit_memcpy), which also gains the DTS bounds macros it was missing. unit-update-disk-fit (drives the real update_disk.c wolfBoot_start) gains two cases: a parsed size above WOLFBOOT_DTS_MAX_SIZE and one below WOLFBOOT_DTS_MIN_SIZE are both rejected without a copy, while the existing success/failure-copy cases keep passing. The staging stand-in is grown so the pre-fix unbounded copy is observable as a copy instead of a crash. --- src/update_disk.c | 24 ++++++++-- src/update_ram.c | 13 ++++-- tools/unit-tests/unit-update-disk-fit.c | 58 ++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/update_disk.c b/src/update_disk.c index edc5799f..e371925a 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -249,6 +249,17 @@ static void disk_decrypted_header_clear(uint8_t *hdr) extern int wolfBoot_get_dts_size(void *dts_addr); +#if defined(MMU) || defined(WOLFBOOT_FDT) +/* Bounds for the attacker-influenced fdt_totalsize before relocating a DTB. + * MIN is the FDT v17 header size (also enforced by the signer): fdt_check_header + * validates magic/version but not totalsize, so a crafted header with a tiny + * totalsize must be rejected rather than loaded/forwarded as a partial tree. */ +#ifndef WOLFBOOT_DTS_MAX_SIZE +#define WOLFBOOT_DTS_MAX_SIZE (1024U * 1024U) +#endif +#define WOLFBOOT_DTS_MIN_SIZE (40U) +#endif + #if defined(WOLFBOOT_NO_LOAD_ADDRESS) || !defined(WOLFBOOT_LOAD_ADDRESS) /* from the linker, where wolfBoot ends */ extern uint8_t _end_wb[]; @@ -632,10 +643,17 @@ void RAMFUNCTION wolfBoot_start(void) } #endif if (flat_dt != NULL) { - uint8_t *dts_ptr = fit_load_image(fit, flat_dt, (int*)&dts_size); - if (dts_ptr != NULL && wolfBoot_get_dts_size(dts_ptr) >= 0) { - /* relocate to load DTS address */ + uint8_t *dts_ptr = fit_load_image(fit, flat_dt, NULL); + int parsed = (dts_ptr != NULL) + ? wolfBoot_get_dts_size(dts_ptr) : -1; + if (dts_ptr != NULL && + parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && + (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { + /* Relocate to the load DTS address. The copy length is + * the parsed DTB size (bounded by the staging region), + * not the FIT-declared property length. */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; + dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", dts_ptr, dts_addr, dts_size); if (wolfBoot_fit_memcpy(dts_addr, dts_ptr, dts_size) != 0) { diff --git a/src/update_ram.c b/src/update_ram.c index 0dac4465..b632b16d 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -655,10 +655,17 @@ backup_on_failure: } #endif if (flat_dt != NULL) { - uint8_t *dts_ptr = fit_load_image(fit, flat_dt, (int*)&dts_size); - if (dts_ptr != NULL && wolfBoot_get_dts_size(dts_ptr) >= 0) { - /* relocate to load DTS address */ + uint8_t *dts_ptr = fit_load_image(fit, flat_dt, NULL); + int parsed = (dts_ptr != NULL) + ? wolfBoot_get_dts_size(dts_ptr) : -1; + if (dts_ptr != NULL && + parsed >= (int)WOLFBOOT_DTS_MIN_SIZE && + (uint32_t)parsed <= WOLFBOOT_DTS_MAX_SIZE) { + /* Relocate to the load DTS address. The copy length is + * the parsed DTB size (bounded by the staging region), + * not the FIT-declared property length. */ dts_addr = (uint8_t*)WOLFBOOT_LOAD_DTS_ADDRESS; + dts_size = (uint32_t)parsed; wolfBoot_printf("Loading DTS: %p -> %p (%d bytes)\n", dts_ptr, dts_addr, dts_size); memcpy(dts_addr, dts_ptr, dts_size); diff --git a/tools/unit-tests/unit-update-disk-fit.c b/tools/unit-tests/unit-update-disk-fit.c index 5799ada7..0b401d8d 100644 --- a/tools/unit-tests/unit-update-disk-fit.c +++ b/tools/unit-tests/unit-update-disk-fit.c @@ -54,17 +54,26 @@ #include #define TEST_PAYLOAD_SIZE 64 -#define TEST_DTS_SIZE 32 +/* A plausible DTB size: at least WOLFBOOT_DTS_MIN_SIZE (the 40-byte + * FDT v17 header). */ +#define TEST_DTS_SIZE 48 +/* Staging-region stand-in. The oversized test relies on it being big + * enough that the pre-fix unbounded copy lands fully inside it, so the + * regression is observable as "a copy happened" rather than a crash. */ +#define TEST_DTS_STAGE_SIZE (2U * 1024U * 1024U) static uint8_t load_buffer[TEST_PAYLOAD_SIZE]; #define WOLFBOOT_LOAD_ADDRESS ((uintptr_t)load_buffer) -static uint8_t dts_buffer[TEST_DTS_SIZE]; +static uint8_t dts_buffer[TEST_DTS_STAGE_SIZE]; #define WOLFBOOT_LOAD_DTS_ADDRESS ((uintptr_t)dts_buffer) static uint8_t part_a_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; static uint8_t part_b_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; -static uint8_t fit_dts_image[TEST_DTS_SIZE]; +static uint8_t fit_dts_image[TEST_DTS_STAGE_SIZE]; +/* Parsed DTB size the wolfBoot_get_dts_size() stub reports, and (pre + * fix) the FIT-declared length the fit_load_image() stub returns. */ +static int mock_dts_size; static int mock_do_boot_called; static int mock_fit_memcpy_ret; static int mock_fit_memcpy_called; @@ -107,6 +116,7 @@ static void reset_mocks(void) build_image(part_a_image, 1, 0xA1); build_image(part_b_image, 2, 0xB2); memset(fit_dts_image, 0xDD, sizeof(fit_dts_image)); + mock_dts_size = TEST_DTS_SIZE; mock_do_boot_called = 0; mock_fit_memcpy_ret = 0; mock_fit_memcpy_called = 0; @@ -217,11 +227,12 @@ int wolfBoot_verify_authenticity(struct wolfBoot_image* img) } /* The loaded payload is treated as a FIT container, and the sub-image - * returned by fit_load_image() is a valid flat device tree. */ + * returned by fit_load_image() is a flat device tree whose parsed + * size is mock_dts_size. */ int wolfBoot_get_dts_size(void *dts_addr) { (void)dts_addr; - return TEST_DTS_SIZE; + return mock_dts_size; } /* Only reached through the fdt_version()/fdt_totalsize() trace macros here. */ @@ -251,7 +262,7 @@ void* fit_load_image(void* fdt, const char* image, int* lenp) (void)fdt; (void)image; if (lenp != NULL) - *lenp = TEST_DTS_SIZE; + *lenp = mock_dts_size; return fit_dts_image; } @@ -331,12 +342,47 @@ START_TEST(test_update_disk_fit_dts_copy_success_boots) } END_TEST +/* A parsed DTB larger than the staging bound (WOLFBOOT_DTS_MAX_SIZE) + * must be rejected rather than copied: before the fix the copy length + * came from the FIT-declared property length, unbounded against the + * staging region. */ +START_TEST(test_update_disk_fit_dts_oversized_rejected) +{ + reset_mocks(); + mock_dts_size = (1024 * 1024) + 4; /* > WOLFBOOT_DTS_MAX_SIZE */ + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + /* nothing may have been copied into the staging region */ + ck_assert_uint_eq(dts_buffer[0], 0); +} +END_TEST + +/* A parsed DTB smaller than the FDT header size is a partial tree and + * must be rejected. */ +START_TEST(test_update_disk_fit_dts_below_min_rejected) +{ + reset_mocks(); + mock_dts_size = 32; /* < WOLFBOOT_DTS_MIN_SIZE (40) */ + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + ck_assert_uint_eq(dts_buffer[0], 0); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("wolfBoot"); TCase *tc = tcase_create("update-disk-fit"); tcase_add_test(tc, test_update_disk_fit_dts_copy_failure_zeroizes_key_material); + tcase_add_test(tc, test_update_disk_fit_dts_oversized_rejected); + tcase_add_test(tc, test_update_disk_fit_dts_below_min_rejected); tcase_add_test(tc, test_update_disk_fit_dts_copy_success_boots); suite_add_tcase(s, tc);