mirror of https://github.com/wolfSSL/wolfBoot.git
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.pull/788/head
parent
a8a9eec96b
commit
2766450123
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue