Multiple fixes: buffer bound checks

[danielinux]

83091e5 TPM: RoT comparison made Constant time F/448
1b54481 TPM NV blob functions: Limit authsz to buffer capacity F/375
73a10e5 Added bound check to get_decrypted_blob_version F/374
927b8c9 Force-zero secrets in update_disk.c F/97
ff20310 strings.c: enforce null-termination on strncpy, optimize strcat F/439 and F/440
45eaf6f Propagate hal_flash_write() error when writing partition magic F/437
9194129 key_sha384: zero hash buffer to cover for early error F/370
3153775 Fix xmalloc bucket size issue with "SPMATH=1 WOLFBOOT_SMALL_STACK=1" F/366
1c6ee12 Fix regression in WOLFBOOT_RAMBOOT_MAX_SIZE
d3ff22e wolfBoot_unseal_blob: ensure secret is zeroed from stack after use F/444
3e07cbe wolfBoot_unseal_auth(): secretSz is now in/out param F/442
0365eb7 Check fw_size when WOLFBOOT_FIXED_PARTITIONS is off F/363
68a1b01 Validate blob size from TPM NV storage F/372
45749ed STM32H7: Fix wrong block size in flash ops F/367

[Copilot]

Pull request overview

This PR hardens several code paths against out-of-bounds reads/writes and secret leakage, and adds/extends unit tests to lock the behavior in.

Changes:

• Add bound/capacity checks for TPM NV blob reads, unseal output size, and encrypted disk header TLVs.
• Zeroize sensitive buffers across more failure paths (disk encryption, TPM unseal, key hashing).
• Improve tests and build matrix to cover the new checks and configurations (no partitions, SHA-384, SHA3-384, TPM blob, update-disk).

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tools/unit-tests/unit-update-disk.c	New unit tests for update-disk encryption failure/boot paths and TLV truncation handling
tools/unit-tests/unit-tpm-rsa-exp.c	Add test ensuring RoT digest comparison avoids memcmp (constant-time compare)
tools/unit-tests/unit-tpm-blob.c	New unit tests for TPM blob NV bounds/auth checks and unseal capacity handling
tools/unit-tests/unit-string.c	Extend tests for strncpy padding / termination behavior
tools/unit-tests/unit-nvm.c	Add test ensuring flash-write error stops partition-magic update
tools/unit-tests/unit-mock-flash.c	Add injectable failure path for hal_flash_write() in unit tests
tools/unit-tests/unit-image.c	Expand hashing/config coverage; add RAM-boot size cap test without partitions
tools/unit-tests/Makefile	Add new test targets (update-disk, TPM blob, no partitions, SHA-384, SHA3-384)
src/xmalloc.c	Fix xmalloc pool sizing for small-stack / SPMATH configurations
src/x86/ahci.c	Initialize unseal secret-size parameter correctly before use
src/update_flash.c	Initialize unseal secret-size parameters correctly before use
src/update_disk.c	Add TLV bounds check and force-zero disk encryption key/nonce across failure + pre-boot
src/tpm.c	Add auth-size caps, NV blob size bounds checks, unseal capacity checks, constant-time digest compare
src/string.c	Avoid repeated strlen in strcat; add zero-padding behavior to strncpy
src/libwolfboot.c	Propagate hal_flash_write() failure from partition_magic_write()
src/image.c	Zero hash buffers on early return; cap fw_size in RAM-boot mode (no partitions)
include/target.h.in	Make RAM-boot max-size define optional via generated define block
hal/stm32h7.c	Adjust bank argument usage in flash wait/unlock/lock flow
docs/TPM.md	Document wolfBoot_unseal_auth() secret_sz as in/out capacity/result
Makefile	Generate optional WOLFBOOT_RAMBOOT_MAX_SIZE define in target.h
.github/workflows/test-library.yml	Remove matrix exclusions now that the underlying issue is fixed

Comments suppressed due to low confidence (1)

src/string.c:180

• The PR description says strncpy now enforces null-termination, but this implementation only zero-pads after encountering a \\0 in src—it still does not guarantee a terminator when truncating (src length >= n), which matches standard strncpy. Either update the PR description to reflect the actual change (zero-padding behavior) or intentionally enforce termination (with clear documentation of the nonstandard semantics).

char *strncpy(char *dst, const char *src, size_t n)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

hal_flash_write_fail is declared static, but tools/unit-tests/unit-nvm.c assigns to hal_flash_write_fail = 1;. With internal linkage, this will fail to link (or won’t affect the variable used by hal_flash_write). Make this non-static and declare it extern where used, or prefer a small setter API (e.g., unit_mock_flash_set_write_fail(int)) to avoid cross-test globals.

Suggested change

	static int hal_flash_write_fail = 0;
	int hal_flash_write_fail = 0;

[Copilot]

This bounds check performs pointer arithmetic that can be undefined if 4 + tlv_len overflows the remaining range (pointer overflow is UB even if you immediately compare). Prefer a size-based check (e.g., compute remaining = (size_t)(max_p - p) and ensure remaining >= 4 and remaining - 4 >= tlv_len) before accessing p + 4.

[Copilot]

In the mock disk_part_read, off + sz can overflow uint64_t, which can bypass the bounds check and allow out-of-range memcpy. Use an overflow-safe check pattern (e.g., if (off > max || sz > max - off)) where max = IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE.

Suggested change

	(void)drv;
	image = (part == BOOT_PART_B) ? part_b_image : part_a_image;
	if (off + sz > IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE)
	const uint64_t max = (uint64_t)IMAGE_HEADER_SIZE + (uint64_t)TEST_PAYLOAD_SIZE;
	(void)drv;
	image = (part == BOOT_PART_B) ? part_b_image : part_a_image;
	if (off > max || sz > max - off)

[Copilot]

These casts can trigger unaligned loads and strict-aliasing/UB (especially on non-x86 targets). Even though this is unit-test code, it’s easy to keep it portable by reading via memcpy into a uint32_t (or using an explicit little-endian helper) instead of dereferencing uint32_t* from a byte buffer.

Suggested change

	int wolfBoot_open_image_address(struct wolfBoot_image* img, uint8_t* image)
	{
	uint32_t magic = *(uint32_t *)image;
	if (magic != WOLFBOOT_MAGIC)
	return -1;
	memset(img, 0, sizeof(*img));
	img->hdr = image;
	img->fw_size = *(uint32_t *)(image + sizeof(uint32_t));
	static uint32_t get_u32_le(const uint8_t *src)
	{
	return (uint32_t)src[0] |
	((uint32_t)src[1] << 8) |
	((uint32_t)src[2] << 16) |
	((uint32_t)src[3] << 24);
	}
	int wolfBoot_open_image_address(struct wolfBoot_image* img, uint8_t* image)
	{
	uint32_t magic = get_u32_le(image);
	if (magic != WOLFBOOT_MAGIC)
	return -1;
	memset(img, 0, sizeof(*img));
	img->hdr = image;
	img->fw_size = get_u32_le(image + sizeof(uint32_t));

[Copilot]

These casts can trigger unaligned loads and strict-aliasing/UB (especially on non-x86 targets). Even though this is unit-test code, it’s easy to keep it portable by reading via memcpy into a uint32_t (or using an explicit little-endian helper) instead of dereferencing uint32_t* from a byte buffer.

Suggested change

	uint32_t magic = *(uint32_t *)image;
	if (magic != WOLFBOOT_MAGIC)
	return -1;
	memset(img, 0, sizeof(*img));
	img->hdr = image;
	img->fw_size = *(uint32_t *)(image + sizeof(uint32_t));
	uint32_t magic;
	uint32_t fw_size;
	memcpy(&magic, image, sizeof(magic));
	if (magic != WOLFBOOT_MAGIC)
	return -1;
	memset(img, 0, sizeof(*img));
	img->hdr = image;
	memcpy(&fw_size, image + sizeof(uint32_t), sizeof(fw_size));
	img->fw_size = fw_size;

[Copilot]

These unaligned uint16_t* stores into dataBuf are undefined on architectures requiring alignment. Use memcpy(dataBuf, &value, sizeof(value)) to write the 16-bit length field.

[Copilot]

These unaligned uint16_t* stores into dataBuf are undefined on architectures requiring alignment. Use memcpy(dataBuf, &value, sizeof(value)) to write the 16-bit length field.

[Copilot]

The format string uses %d but the arguments are cast to unsigned int. For correctness (and to avoid confusing negative output for large values), use %u (or a fixed-width format via PRIu32 with an appropriate cast).

[Copilot]

The PR description says strncpy now enforces null-termination, but this implementation only zero-pads after encountering a \\0 in src—it still does not guarantee a terminator when truncating (src length >= n), which matches standard strncpy. Either update the PR description to reflect the actual change (zero-padding behavior) or intentionally enforce termination (with clear documentation of the nonstandard semantics).

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

The new bounds check uses pointer arithmetic (p + 4 + tlv_len) that can invoke undefined behavior when tlv_len is attacker-controlled and large enough to move the pointer far past the object (even before the comparison). Consider rewriting this check using integer remaining-length math (e.g., compare (size_t)(max_p - p) against 4 + tlv_len) to avoid UB while keeping the same behavior.

[Copilot]

These unit-test mocks write uint16_t values via *(uint16_t*)dataBuf, which can be unaligned and violates strict-aliasing rules. This is undefined behavior on some architectures and can break under sanitizers. Prefer writing the 16-bit value via memcpy (or a small helper that writes bytes explicitly) to keep the test portable.

[Copilot]

hal_flash_write_fail is introduced as static in unit-mock-flash.c but is directly assigned here, which makes the build fragile (it only works if unit-mock-flash.c is included into this translation unit in the right order). Consider exposing a small setter API (e.g., mock_hal_flash_write_set_fail_once(int)) or making the flag non-static with an extern declaration in a shared test header so the linkage and intent are explicit.