From 0412238414147fd6eab78aaa61245236871feba5 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Mon, 24 Aug 2026 19:56:10 +0200 Subject: [PATCH] F-11044: stage the fwTPM command in secure memory before processing wcs_fwtpm_transmit() verified that cmd identifies non-secure memory and then passed that mutable buffer directly to FWTPM_ProcessCommand. The processor parses the packet more than once (authentication, then handler execution), so a DMA-capable non-secure attacker who rewrites the command buffer in the window between the two parses can make the authenticated command differ from the executed command. Copy exactly cmdSz bytes into a secure staging buffer after the range validation and invoke the processor only on that copy: an NS DMA master cannot rewrite secure memory, so both parses see the same bytes. The command and response staging are zeroed before returning (the response may carry auth tags or unsealed data). unit-fwtpm-cmd-toctou includes the real fwtpm_callable.c and mocks FWTPM_ProcessCommand with two parse points (authentication, execution). The test plays the attacker, rewriting the NS command buffer at the window between the parses - the mock may only touch the caller's buffer, never secure staging, mirroring the hardware boundary. Pre-fix the processor authenticated the original bytes and executed the rewritten ones (test fails); post-fix both parses see the original command. Verification: - Built: unit tests compile the real veneer (host, poisoned fwtpm headers, same pattern as unit-fwtpm-nv-oob). - Tested: unit-fwtpm-cmd-toctou 2/2 (red demonstrated against the pre-fix veneer via git stash of the fix); sibling unit-fwtpm-rsp-overrun 3/3 and unit-fwtpm-nv-oob 4/4 after the change. - Pitfalls: the staging copy happens after all NS range checks and before any processor access; zeroing covers the full staging buffers regardless of the produced length. - Style: cstyle-check.sh flag count on src/fwtpm_callable.c unchanged (1 pre-existing); the new test trips only the uncrustify pointer-alignment class the sibling unit tests trip. - Message: F-11044: prefix, no co-author trailers. - Unverified: no CMSE/armclang build and no m33mu emulator run here (lib/wolftpm is not checked out in this tree); the trustzone-emulator workflow covers the full build on push. --- src/fwtpm_callable.c | 16 +- tools/unit-tests/Makefile | 8 + tools/unit-tests/unit-fwtpm-cmd-toctou.c | 207 +++++++++++++++++++++++ 3 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 tools/unit-tests/unit-fwtpm-cmd-toctou.c diff --git a/src/fwtpm_callable.c b/src/fwtpm_callable.c index 80dc5ee5..b1adb484 100644 --- a/src/fwtpm_callable.c +++ b/src/fwtpm_callable.c @@ -49,6 +49,12 @@ static int fwtpm_ready; * checked against the snapshotted capacity. */ static uint8_t g_fwtpm_rsp_stage[WCS_FWTPM_MAX_COMMAND_SIZE]; +/* Command staging: the processor parses the packet more than once + * (authentication, then execution), and a DMA-capable non-secure + * attacker cannot rewrite secure memory, so both parses see the same + * bytes even if the NS command buffer changes in between. */ +static uint8_t g_fwtpm_cmd_stage[WCS_FWTPM_MAX_COMMAND_SIZE]; + extern unsigned int _start_heap; extern unsigned int _heap_size; @@ -171,8 +177,12 @@ int CSME_NSE_API wcs_fwtpm_transmit(const uint8_t *cmd, uint32_t cmdSz, return BAD_FUNC_ARG; } + /* Stage the command before invoking the processor (see the staging + * buffer comment above). */ + memcpy(g_fwtpm_cmd_stage, cmd, cmdSz); + rspLen = (int)WCS_FWTPM_MAX_COMMAND_SIZE; - rc = FWTPM_ProcessCommand(&fwtpm_ctx, cmd, (int)cmdSz, + rc = FWTPM_ProcessCommand(&fwtpm_ctx, g_fwtpm_cmd_stage, (int)cmdSz, g_fwtpm_rsp_stage, &rspLen, 0); if (rc == TPM_RC_SUCCESS) { wireSz = fwtpm_rsp_size(g_fwtpm_rsp_stage, rspLen); @@ -188,6 +198,10 @@ int CSME_NSE_API wcs_fwtpm_transmit(const uint8_t *cmd, uint32_t cmdSz, rc = TPM_RC_FAILURE; } } + /* Zero the staging before returning: the response may carry + * sensitive material (auth tags, unsealed data). */ + memset(g_fwtpm_cmd_stage, 0, sizeof(g_fwtpm_cmd_stage)); + memset(g_fwtpm_rsp_stage, 0, sizeof(g_fwtpm_rsp_stage)); return rc; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 63828430..a4442b8c 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -117,6 +117,7 @@ TESTS+=unit-t10xx-flash-status TESTS+=unit-p1021-erase-advance TESTS+=unit-hifive1-flash-write TESTS+=unit-fwtpm-rsp-overrun +TESTS+=unit-fwtpm-cmd-toctou TESTS+=unit-aurix-erased-fill TESTS+=unit-aurix-erased-fill-invert TESTS+=unit-t2080-fman-loader @@ -385,6 +386,13 @@ unit-fwtpm-rsp-overrun: ../../include/target.h unit-fwtpm-rsp-overrun.c gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ -DWOLFTPM_USER_SETTINGS $(LDFLAGS) +# unit-fwtpm-cmd-toctou: a midflight NS rewrite of the command buffer +# must not change what the processor executes (F-11044); the veneer +# stages the command in secure memory before processing. +unit-fwtpm-cmd-toctou: ../../include/target.h unit-fwtpm-cmd-toctou.c + gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) \ + -DWOLFTPM_USER_SETTINGS $(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-fwtpm-cmd-toctou.c b/tools/unit-tests/unit-fwtpm-cmd-toctou.c new file mode 100644 index 00000000..6d7bf728 --- /dev/null +++ b/tools/unit-tests/unit-fwtpm-cmd-toctou.c @@ -0,0 +1,207 @@ +/* unit-fwtpm-cmd-toctou.c + * + * Regression test for F-11044: wcs_fwtpm_transmit() validated that cmd + * identifies non-secure memory and then passed that mutable buffer + * directly to FWTPM_ProcessCommand. The processor parses the packet + * more than once (authentication, then execution), so a DMA-capable + * non-secure attacker who rewrites the command buffer in between can + * make the authenticated command differ from the executed command. + * + * The fix copies the command into secure staging memory after the + * range validation and invokes the processor only on that copy, which + * an NS DMA master cannot touch. + * + * The real fwtpm_callable.c is included; FWTPM_ProcessCommand is + * mocked with two parse points (authentication, execution) and the + * test plays the attacker, rewriting the NS command buffer at the + * window between them. The mock may only rewrite the caller's buffer, + * never the secure staging, mirroring the hardware boundary. + * + * 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 + +#define WOLFBOOT_TZ_FWTPM + +/* Minimal types/macros that fwtpm_callable.c uses from wolftpm headers. + * Poison the include guards of the heavy fwtpm headers so they become + * no-ops; we define the handful of types/symbols we actually need + * ourselves (same pattern as unit-fwtpm-nv-oob). */ +#define _FWTPM_H_ +#define _FWTPM_COMMAND_H_ +#define _FWTPM_NV_H_ + +#define WOLFTPM2_NO_WOLFCRYPT + +typedef uint8_t byte; +typedef uint32_t word32; + +#define BAD_FUNC_ARG (-173) +#define TPM_RC_SUCCESS 0x000 +#define TPM_RC_FAILURE 0x101 +#define TPM_RC_INITIALIZE 0x100 + +#define XMEMCPY(d,s,l) memcpy((d),(s),(l)) +#define XMEMSET(b,c,l) memset((b),(c),(l)) + +#define TPM2_HEADER_SIZE 10 + +typedef struct FWTPM_NV_HAL_S { + int (*read)(void *ctx, word32 offset, byte *buf, word32 size); + int (*write)(void *ctx, word32 offset, const byte *buf, word32 size); + int (*erase)(void *ctx, word32 offset, word32 size); + void *ctx; + word32 maxSize; +} FWTPM_NV_HAL; + +typedef struct FWTPM_CTX_S { + int wasStarted; +} FWTPM_CTX; + +/* Stub symbols referenced by fwtpm_callable.c */ +unsigned int _start_heap; +unsigned int _heap_size; + +void *wolfboot_store_sbrk(unsigned int incr, uint8_t **heap, + uint8_t *start, uint32_t size) +{ + (void)incr; (void)heap; (void)start; (void)size; + return NULL; +} + +int FWTPM_Init(FWTPM_CTX *ctx) { (void)ctx; return 0; } +int FWTPM_NV_SetHAL(FWTPM_CTX *ctx, FWTPM_NV_HAL *hal) + { (void)ctx; (void)hal; return 0; } + +static const uint8_t g_err_rsp[TPM2_HEADER_SIZE] = { + 0x80, 0xC1, 0x00, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x00, 0x09 +}; + +#define CMD_SZ 24 +static uint8_t g_ns_cmd[CMD_SZ]; /* the non-secure world's command */ + +/* 1 when the attacker fires in the parse window of this call. */ +static int g_attack; + +/* What the processor saw at each parse point. */ +static uint8_t g_authed[CMD_SZ]; +static uint8_t g_executed[CMD_SZ]; + +int FWTPM_ProcessCommand(FWTPM_CTX *ctx, const byte *cmdBuf, int cmdSize, + byte *rspBuf, int *rspSize, int locality) +{ + (void)ctx; (void)locality; + + /* Authentication parse. */ + memcpy(g_authed, cmdBuf, (size_t)cmdSize); + + if (g_attack) { + /* The attacker's DMA rewrite reaches only NS memory - the + * caller's command buffer - never secure staging. */ + memset(g_ns_cmd, 0x5A, (size_t)cmdSize); + } + + /* Execution parse. */ + memcpy(g_executed, cmdBuf, (size_t)cmdSize); + + memcpy(rspBuf, g_err_rsp, TPM2_HEADER_SIZE); + *rspSize = TPM2_HEADER_SIZE; + return TPM_RC_SUCCESS; +} + +/* Bring in the code under test as part of this translation unit. */ +#include "../../src/fwtpm_callable.c" + +/* With the attacker firing mid-processing, the command the processor + * authenticates must be byte-identical to the command it executes: + * pre-fix the processor parses the caller's mutable buffer, so the + * rewrite changes the executed command. */ +START_TEST(cmd_immune_to_midflight_rewrite) +{ + uint8_t rsp[16]; + uint32_t rspSz = sizeof(rsp); + int i; + int rc; + + for (i = 0; i < CMD_SZ; i++) + g_ns_cmd[i] = (uint8_t)(0x10 + i); + + wcs_fwtpm_init(); + g_attack = 1; + + rc = wcs_fwtpm_transmit(g_ns_cmd, CMD_SZ, rsp, &rspSz); + ck_assert_int_eq(rc, TPM_RC_SUCCESS); + ck_assert_uint_eq(rspSz, TPM2_HEADER_SIZE); + + ck_assert_mem_eq(g_authed, g_executed, CMD_SZ); + /* The attacker did rewrite the NS buffer (the attack model fired); + * the processor just did not act on it. */ + ck_assert_uint_eq(g_ns_cmd[0], 0x5A); + for (i = 0; i < CMD_SZ; i++) + ck_assert_uint_eq(g_executed[i], (uint8_t)(0x10 + i)); +} +END_TEST + +/* No attacker: an ordinary transmission is unaffected by the staging. */ +START_TEST(cmd_plain_transmission_intact) +{ + uint8_t rsp[16]; + uint32_t rspSz = sizeof(rsp); + int i; + int rc; + + for (i = 0; i < CMD_SZ; i++) + g_ns_cmd[i] = (uint8_t)(0x40 + i); + + wcs_fwtpm_init(); + g_attack = 0; + + rc = wcs_fwtpm_transmit(g_ns_cmd, CMD_SZ, rsp, &rspSz); + ck_assert_int_eq(rc, TPM_RC_SUCCESS); + ck_assert_uint_eq(rspSz, TPM2_HEADER_SIZE); + ck_assert_mem_eq(g_authed, g_executed, CMD_SZ); + for (i = 0; i < CMD_SZ; i++) + ck_assert_uint_eq(g_executed[i], (uint8_t)(0x40 + i)); +} +END_TEST + +static Suite *fwtpm_cmd_toctou_suite(void) +{ + Suite *s = suite_create("fwtpm_cmd_toctou"); + TCase *tc = tcase_create("midflight-rewrite"); + + tcase_add_test(tc, cmd_immune_to_midflight_rewrite); + tcase_add_test(tc, cmd_plain_transmission_intact); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = fwtpm_cmd_toctou_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails == 0 ? 0 : 1; +}