From 27664501231dc30e240e503f787d0b3518156fea Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Fri, 5 Jun 2026 19:26:28 +0200 Subject: [PATCH] F-4710: reject oversized FDT property length to prevent ~4GB FIT memcpy fdt_next_tag() advanced its struct cursor with offset += sizeof(struct fdt_property) - FDT_TAGSIZE + fdt32_to_cpu(*lenp); using unsigned arithmetic. A property whose len field is 0xFFFFFFFF wrapped this to a +7 advance, so the malformed node slipped past the fdt_offset_ptr() bounds check at the end of fdt_next_tag(). The bogus length then propagated up through fdt_get_property_by_offset() / fdt_getprop() and was returned by fit_load_image() as *lenp = -1. In the MMU FIT boot path, wolfBoot_start() (src/update_ram.c:465) aliases that out-parameter through (int*)&dts_size, turning -1 into a uint32_t 0xFFFFFFFF, and the only guard before the relocation is dts_ptr != NULL, so memcpy(WOLFBOOT_LOAD_DTS_ADDRESS, dts_ptr, 0xFFFFFFFF) ran (CWE-680). A FIT subimage with no "load" property reaches this with the inner copy skipped, so the giant size hits the outer DTS relocation directly. Fix at the root: a property value can never exceed the blob, so reject any FDT_PROP whose declared length is greater than fdt_totalsize() before the cursor arithmetic. This closes the wrap for every caller of fdt_next_tag() (including the other fit_load_image() sinks), not just the DTS path. Legitimate properties (len <= size_dt_struct < totalsize) are unaffected. Add a regression test to unit-fdt: a hand-built FIT whose /images/kernel-1 "data" property declares len=0xFFFFFFFF must make fit_load_image_ex() fail closed (return NULL) instead of handing back a live pointer with a negative length. The test fails before this change and passes after. --- src/fdt.c | 9 ++++++ tools/unit-tests/unit-fdt.c | 59 +++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/fdt.c b/src/fdt.c index b31feee9..095653c7 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -152,6 +152,15 @@ static uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) if (!lenp) { return FDT_END; /* premature end */ } + /* A property value can never be larger than the blob itself. + * Reject an oversized length up front: otherwise the unsigned + * cursor arithmetic below wraps (e.g. len=0xFFFFFFFF advances + * offset by only 7 bytes), the malformed node slips past the + * fdt_offset_ptr() bounds check, and the bogus length propagates + * to callers as a negative int (a ~4GB memcpy size). */ + if (fdt32_to_cpu(*lenp) > (uint32_t)fdt_totalsize(fdt)) { + return FDT_END; /* bad structure */ + } /* skip-name offset, length and value */ offset += sizeof(struct fdt_property) - FDT_TAGSIZE + fdt32_to_cpu(*lenp); diff --git a/tools/unit-tests/unit-fdt.c b/tools/unit-tests/unit-fdt.c index c52672fe..96f9e6e2 100644 --- a/tools/unit-tests/unit-fdt.c +++ b/tools/unit-tests/unit-fdt.c @@ -78,6 +78,64 @@ START_TEST(test_fdt_get_string_returns_string_with_valid_offset) } END_TEST +/* Minimal FIT with a single /images/kernel-1 node whose `data` property + * declares len=0xFFFFFFFF. There is no `load` (and no `compression`), so + * fit_load_image_inner() takes the pass-through branch. Before the + * fdt_next_tag() length check, the oversized len wrapped the cursor + * arithmetic, slipped past the bounds check, and was handed back as + * *lenp = -1 - which update_ram.c then aliased into a ~4GB memcpy size. + * The loader must instead fail closed (return NULL). */ +static const uint8_t fit_data_len_overflow[] = { + /* header */ + 0xd0, 0x0d, 0xfe, 0xed, /* magic */ + 0x00, 0x00, 0x00, 0x81, /* totalsize = 129 */ + 0x00, 0x00, 0x00, 0x38, /* off_dt_struct = 56 */ + 0x00, 0x00, 0x00, 0x7c, /* off_dt_strings = 124 */ + 0x00, 0x00, 0x00, 0x28, /* off_mem_rsvmap = 40 */ + 0x00, 0x00, 0x00, 0x11, /* version = 17 */ + 0x00, 0x00, 0x00, 0x10, /* last_comp_version = 16 */ + 0x00, 0x00, 0x00, 0x00, /* boot_cpuid_phys */ + 0x00, 0x00, 0x00, 0x05, /* size_dt_strings = 5 */ + 0x00, 0x00, 0x00, 0x44, /* size_dt_struct = 68 */ + /* mem_rsvmap terminator (offset 40) */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* struct block (offset 56) */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE root */ + 0x00, 0x00, 0x00, 0x00, /* "" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE images */ + 0x69, 0x6d, 0x61, 0x67, 0x65, 0x73, 0x00, 0x00, /* "images\0\0" */ + 0x00, 0x00, 0x00, 0x01, /* BEGIN_NODE kernel-1 */ + 0x6b, 0x65, 0x72, 0x6e, 0x65, 0x6c, 0x2d, 0x31, + 0x00, 0x00, 0x00, 0x00, /* "kernel-1\0\0\0\0" */ + 0x00, 0x00, 0x00, 0x03, /* FDT_PROP */ + 0xff, 0xff, 0xff, 0xff, /* len = 0xFFFFFFFF */ + 0x00, 0x00, 0x00, 0x00, /* nameoff = 0 ("data") */ + 0x00, 0x00, 0x00, 0x00, /* data (4 bytes) */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE kernel-1 */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE images */ + 0x00, 0x00, 0x00, 0x02, /* END_NODE root */ + 0x00, 0x00, 0x00, 0x09, /* FDT_END */ + /* strings block (offset 124) */ + 0x64, 0x61, 0x74, 0x61, 0x00, /* "data\0" */ +}; + +START_TEST(test_fit_load_image_rejects_oversized_prop_len) +{ + static uint8_t fit_scratch[sizeof(fit_data_len_overflow)]; + int len = 0; + void *ret; + + memcpy(fit_scratch, fit_data_len_overflow, sizeof(fit_scratch)); + + ret = fit_load_image_ex(fit_scratch, "kernel-1", &len, 64 * 1024); + + /* Must fail closed: never return a live pointer with a negative + * length that a caller could turn into a giant memcpy size. */ + ck_assert_ptr_null(ret); +} +END_TEST + static Suite *fdt_suite(void) { Suite *s = suite_create("fdt"); @@ -85,6 +143,7 @@ static Suite *fdt_suite(void) tcase_add_test(tc, test_fdt_get_string_rejects_out_of_range_offset); tcase_add_test(tc, test_fdt_get_string_returns_string_with_valid_offset); + tcase_add_test(tc, test_fit_load_image_rejects_oversized_prop_len); suite_add_tcase(s, tc); return s;