From 1ecf2248c1a00d82f285f536bcfd30e4164bdb0b Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 24 Aug 2026 21:14:39 +0200 Subject: [PATCH] F-11045: validate the FDT layout before the memreserve memmove fdt_add_mem_rsv() added the 32-bit string-block offset and size without overflow checks. A wrapped data_end bypassed the capacity check, and the same wrapped expression derived the memmove length, so a malformed (or attacker-supplied) DTB produced a huge memmove - broad boot-time memory corruption. fdt_check_header validates only magic and version, so raw-DTB callers reach this code with inconsistent layout fields. Compute the block end in 64-bit, validate the layout before touching it (structure block starts after the reserve map terminator, string block after the structure block, shifted layout fits in totalsize), and derive the move length only from the validated 64-bit end. The reserve-map scan bound uses 64-bit arithmetic as well, so a wrapped 32-bit sum cannot pass it. unit-fdt-memrsv-wrap extracts the real fdt_add_mem_rsv (plus the byte-order helpers) and feeds it crafted DTB headers: - a wrapped end below off_dt: pre-fix the memmove length wraps to ~2^32 and the process segfaults; post-fix rejected (-FDT_ERR_NOSPACE) - a wrapped end inside [off_dt, total): pre-fix the layout was accepted (ret == 0) with a silently corrupted FDT; post-fix rejected - a consistent layout: the entry is inserted, the terminator moves down one, structure and string blocks shift by 16 bytes, and the header offsets follow (regression guard, passed pre-fix as well) Verification: - Built: unit test compiles the extracted real function (host). - Tested: unit-fdt-memrsv-wrap 3/3 post-fix; pre-fix (fix stashed) 1 segfault + 1 assertion failure on the wrap cases, valid case passing - red demonstrated on both corruption modes. - Pitfalls: the helper that builds the crafted DTB only writes block contents where the offsets fit the buffer, so the malformed cases cannot corrupt memory in the test itself before reaching the code under test; validation runs before any block access. - Style: cstyle-check.sh flag count on src/fdt.c unchanged (1 pre-existing FMT class); the new test trips only the uncrustify class the sibling unit tests trip. - Message: F-11045: prefix, no co-author trailers. - Unverified: no target build needed (pure C, host-compiled from the real source); fdt.c compiles as part of the normal wolfBoot build paths unchanged. --- src/fdt.c | 20 ++- tools/unit-tests/Makefile | 13 ++ tools/unit-tests/unit-fdt-memrsv-wrap.c | 190 ++++++++++++++++++++++++ 3 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 tools/unit-tests/unit-fdt-memrsv-wrap.c diff --git a/src/fdt.c b/src/fdt.c index b388d0d1..3ae87fa0 100644 --- a/src/fdt.c +++ b/src/fdt.c @@ -864,7 +864,7 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size) uint32_t off_str; uint32_t size_str; uint32_t total; - uint32_t data_end; + uint64_t data_end; uint32_t shift; uint32_t i; @@ -877,9 +877,18 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size) off_str = fdt_off_dt_strings(fdt); size_str = fdt_size_dt_strings(fdt); total = fdt_totalsize(fdt); - data_end = off_str + size_str; + /* 64-bit: a wrapped 32-bit end would slip past the checks below + * and re-wrap into the memmove length. */ + data_end = (uint64_t)off_str + (uint64_t)size_str; shift = (uint32_t)sizeof(struct fdt_reserve_entry); /* 16 */ + /* Validate the layout before using it: the structure block must + * start after the reserve map's terminator, and the string block + * after the structure block. 64-bit comparisons: a wrapped 32-bit + * sum would pass the check. */ + if (((uint64_t)off_rsv + shift > off_dt) || (off_str < off_dt)) { + return -FDT_ERR_BADSTRUCTURE; + } if ((data_end + shift) > total) { return -FDT_ERR_NOSPACE; } @@ -889,14 +898,15 @@ int fdt_add_mem_rsv(void* fdt, uint64_t address, uint64_t size) i = 0; while ((rsv[i].address != 0ULL) || (rsv[i].size != 0ULL)) { i++; - if ((off_rsv + (i + 1U) * shift) > off_dt) { + if (((uint64_t)off_rsv + (uint64_t)(i + 1U) * shift) > off_dt) { return -FDT_ERR_BADSTRUCTURE; } } - /* Shift structure + strings down by 16 bytes. memmove handles overlap. */ + /* Shift structure + strings down by 16 bytes. memmove handles overlap. + * The length comes from the validated 64-bit end. */ memmove(base + off_dt + shift, base + off_dt, - (size_t)((off_str + size_str) - off_dt)); + (size_t)(data_end - off_dt)); /* Insert new entry where the old terminator was, write new terminator. */ rsv[i].address = cpu_to_fdt64(address); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index a4442b8c..b84056f6 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -118,6 +118,7 @@ TESTS+=unit-p1021-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-fwtpm-rsp-overrun TESTS+=unit-fwtpm-cmd-toctou +TESTS+=unit-fdt-memrsv-wrap TESTS+=unit-aurix-erased-fill TESTS+=unit-aurix-erased-fill-invert TESTS+=unit-t2080-fman-loader @@ -393,6 +394,18 @@ unit-fwtpm-cmd-toctou: ../../include/target.h unit-fwtpm-cmd-toctou.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ -DWOLFTPM_USER_SETTINGS $(LDFLAGS) +# unit-fdt-memrsv-wrap: a wrapped string-block end in fdt_add_mem_rsv() +# must be rejected, not turned into a huge memmove (F-11045). +fdt_memrsv_extract.h: ../../src/fdt.c + sed -n '/^uint32_t cpu_to_fdt32/,/^}/p' $< > $@ + sed -n '/^uint64_t cpu_to_fdt64/,/^}/p' $< >> $@ + sed -n '/^uint32_t fdt32_to_cpu/,/^}/p' $< >> $@ + sed -n '/^uint64_t fdt64_to_cpu/,/^}/p' $< >> $@ + sed -n '/^int fdt_add_mem_rsv/,/^}/p' $< >> $@ + +unit-fdt-memrsv-wrap: unit-fdt-memrsv-wrap.c fdt_memrsv_extract.h + gcc -o $@ unit-fdt-memrsv-wrap.c -I../../include $(CFLAGS) $(LDFLAGS) + unit-tpm-blob: ../../include/target.h unit-tpm-blob.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ -DWOLFTPM_USER_SETTINGS -DWOLFBOOT_TPM_SEAL -DWOLFBOOT_SIGN_RSA2048 \ diff --git a/tools/unit-tests/unit-fdt-memrsv-wrap.c b/tools/unit-tests/unit-fdt-memrsv-wrap.c new file mode 100644 index 00000000..f05ae2d6 --- /dev/null +++ b/tools/unit-tests/unit-fdt-memrsv-wrap.c @@ -0,0 +1,190 @@ +/* unit-fdt-memrsv-wrap.c + * + * Regression test for F-11045: fdt_add_mem_rsv() in src/fdt.c added the + * 32-bit string-block offset and size without overflow checks. A + * wrapped data_end bypassed the capacity check, and the same wrapped + * expression derived the memmove length, so a malformed (or + * attacker-supplied) DTB produced a huge memmove - broad boot-time + * memory corruption. fdt_check_header validates only magic and + * version, so raw-DTB callers reach this code with inconsistent + * layout fields. + * + * The fix uses 64-bit arithmetic for the block end, validates the + * ordering of the reserve map / structure / string blocks, and + * derives the move length only after validation. + * + * The real fdt_add_mem_rsv() (plus the byte-order helpers) is + * extracted by the Makefile; the FDT under test is a crafted header + * plus block layout in a static buffer. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include + +#include "fdt.h" + +#define wolfBoot_printf(...) ((void)0) + +/* Byte-order helpers and the code under test, from src/fdt.c + * (extracted by the Makefile). */ +#include "fdt_memrsv_extract.h" + +#define FDT_MAGIC_V 0xD00DFEEDu +#define BUF_SZ 96 +static uint8_t g_buf[BUF_SZ]; + +/* Store a 32-bit field in FDT (big-endian) byte order. */ +static uint32_t put32(uint32_t v) +{ + return fdt32_to_cpu(v); +} + +/* Craft a header with the given layout fields. The blocks are laid + * out as: 40-byte header, reserve map (16-byte terminator), a 4-byte + * structure block, a 4-byte string block, trailing slack. */ +static void make_fdt(uint32_t total, uint32_t off_rsv, uint32_t off_dt, + uint32_t off_str, uint32_t size_str) +{ + struct fdt_header *h = (struct fdt_header *)g_buf; + + memset(g_buf, 0x77, sizeof(g_buf)); + + h->magic = put32(FDT_MAGIC_V); + h->totalsize = put32(total); + h->off_dt_struct = put32(off_dt); + h->off_dt_strings = put32(off_str); + h->off_mem_rsvmap = put32(off_rsv); + h->version = put32(17); + h->last_comp_version = put32(16); + h->boot_cpuid_phys = put32(0); + h->size_dt_strings = put32(size_str); + h->size_dt_struct = put32(0); + + /* Block contents only where they fit: the malformed-layout tests + * carry out-of-buffer offsets, and the code under test must reject + * them before touching the blocks. */ + if (off_rsv + 16 <= BUF_SZ) { + memset(g_buf + off_rsv, 0, 16); + } + if (off_dt + 4 <= BUF_SZ) { + memset(g_buf + off_dt, 0, 4); + } + if (off_str + 2 <= BUF_SZ) { + g_buf[off_str] = 'x'; + g_buf[off_str + 1] = 0; + } +} + +/* off_str + size_str wraps past 2^32 and the wrapped end lands below + * off_dt: pre-fix the memmove length wraps to ~2^32 (crash/corruption), + * post-fix the 64-bit end is far past totalsize and rejected. */ +START_TEST (test_wrap_past_off_dt_rejected) +{ + int ret; + + /* off_str + size_str = 0x100000010 (wraps to 0x10 in 32-bit). */ + make_fdt(0x2000, 40, 0x100, 0xFFFFFFF0u, 0x100u); + + ret = fdt_add_mem_rsv(g_buf, 0x1000, 0x400); + ck_assert_int_ne(ret, 0); +} +END_TEST + +/* The wrapped end lands inside [off_dt, total): pre-fix this passed + * the capacity check, moved 0 bytes, and still published shifted + * header offsets (a silently corrupted FDT). Post-fix it is rejected. */ +START_TEST (test_wrap_inside_range_rejected) +{ + uint32_t off_str_before; + int ret; + + /* off_str + size_str = 0x100000100 (wraps to 0x100 in 32-bit) + * which equals off_dt. */ + make_fdt(0x2000, 40, 0x100, 0xFFFFFF00u, 0x200u); + off_str_before = fdt_off_dt_strings(g_buf); + + ret = fdt_add_mem_rsv(g_buf, 0x1000, 0x400); + ck_assert_int_ne(ret, 0); + ck_assert_uint_eq(fdt_off_dt_strings(g_buf), off_str_before); + ck_assert_uint_eq(fdt_off_dt_struct(g_buf), 0x100u); +} +END_TEST + +/* A consistent layout still accepts the insertion: the entry goes in + * where the terminator was, the terminator moves down one, structure + * and string blocks shift by 16, and the header offsets follow. */ +START_TEST (test_valid_insertion_works) +{ + struct fdt_reserve_entry *rsv; + int i; + int ret; + + /* Layout: header [0,40), rsv map [40,56) terminator, struct + * [56,60), strings [60,64); 32 bytes of slack for the shift. */ + make_fdt(96, 40, 56, 60, 4); + + ret = fdt_add_mem_rsv(g_buf, 0x80000000ULL, 0x400000ULL); + ck_assert_int_eq(ret, 0); + + /* New entry where the terminator was, then the terminator. */ + rsv = (struct fdt_reserve_entry *)(g_buf + 40); + ck_assert_uint_eq(fdt64_to_cpu(rsv[0].address), 0x80000000ULL); + ck_assert_uint_eq(fdt64_to_cpu(rsv[0].size), 0x400000ULL); + ck_assert_uint_eq(fdt64_to_cpu(rsv[1].address), 0ULL); + ck_assert_uint_eq(fdt64_to_cpu(rsv[1].size), 0ULL); + + /* Header offsets shifted by one reserve entry (16 bytes). */ + ck_assert_uint_eq(fdt_off_dt_struct(g_buf), 56 + 16); + ck_assert_uint_eq(fdt_off_dt_strings(g_buf), 60 + 16); + + /* Structure block (4-byte end marker) and string block moved intact. */ + for (i = 72; i < 76; i++) + ck_assert_uint_eq(g_buf[i], 0); + ck_assert_uint_eq(g_buf[76], 'x'); + ck_assert_uint_eq(g_buf[77], 0); +} +END_TEST + +Suite *fdt_memrsv_wrap_suite(void) +{ + Suite *s = suite_create("fdt memrsv wrap"); + TCase *tc = tcase_create("layout-validation"); + + tcase_add_test(tc, test_wrap_past_off_dt_rejected); + tcase_add_test(tc, test_wrap_inside_range_rejected); + tcase_add_test(tc, test_valid_insertion_works); + tcase_set_timeout(tc, 10); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = fdt_memrsv_wrap_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +}