mirror of https://github.com/wolfSSL/wolfBoot.git
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.pull/868/head
parent
fe1dcb080d
commit
d554bbb98b
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -54,17 +54,26 @@
|
|||
#include <wolfssl/wolfcrypt/chacha.h>
|
||||
|
||||
#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);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue