From 740383a3f53b932dca2ed1ae3e97274805bd0573 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 9 Jun 2026 10:19:01 +0200 Subject: [PATCH] F-4790: clamp OTP pubkey_size in keystore_get_size to prevent OOB read keystore_get_size() returned slot->pubkey_size verbatim from the OTP keystore slot with no upper bound. A corrupted or mis-provisioned slot with pubkey_size > KEYSTORE_PUBKEY_SIZE produces a positive value that passes every caller guard (pubkey_sz < 0 / <= 0). The callers in image.c (key_sha256/key_sha384/key_sha3_384 and the ECC verify y-coordinate offset) then read past otp_slot_item_cache, which only holds KEYSTORE_PUBKEY_SIZE pubkey bytes. Reject an out-of-range pubkey_size by returning -1, matching the existing defensive validation of item_count in keystore_num_pubkeys() and the -1 error convention the callers already handle. Add unit-otp-keystore, which compiles flash_otp_keystore.c in isolation and verifies keystore_get_size() rejects oversized slots. --- src/flash_otp_keystore.c | 10 +++ tools/unit-tests/Makefile | 7 ++ tools/unit-tests/unit-otp-keystore.c | 126 +++++++++++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 tools/unit-tests/unit-otp-keystore.c diff --git a/src/flash_otp_keystore.c b/src/flash_otp_keystore.c index a66eb809..37e0a53f 100644 --- a/src/flash_otp_keystore.c +++ b/src/flash_otp_keystore.c @@ -24,9 +24,13 @@ #include #include +#ifndef WOLFBOOT_UNIT_TEST_OTP_KEYSTORE #include "wolfboot/wolfboot.h" +#endif #include "keystore.h" +#ifndef WOLFBOOT_UNIT_TEST_OTP_KEYSTORE #include "hal.h" +#endif #include "otp_keystore.h" #if defined(FLASH_OTP_KEYSTORE) && !defined(WOLFBOOT_NO_SIGN) @@ -70,6 +74,12 @@ int keystore_get_size(int id) SIZEOF_KEYSTORE_SLOT) != 0) return -1; slot = (struct keystore_slot *)otp_slot_item_cache; + /* The pubkey is cached in a fixed-size buffer holding at most + * KEYSTORE_PUBKEY_SIZE bytes. A larger pubkey_size read from a corrupted or + * mis-provisioned OTP slot would make callers read past the buffer, so + * reject it like other invalid OTP fields. */ + if (slot->pubkey_size > KEYSTORE_PUBKEY_SIZE) + return -1; return slot->pubkey_size; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index a08180b9..6bd15d38 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -63,6 +63,7 @@ TESTS+=unit-fit-gzip unit-fit-nogzip TESTS+=unit-fit-fpga TESTS+=unit-mpusize TESTS+=unit-flash-erase-h7 +TESTS+=unit-otp-keystore # linux_loader.c is x86 32-bit only, so its unit tests need a working 32-bit # (multilib) toolchain. Probe whether "gcc -m32" can link, and only add the @@ -268,6 +269,12 @@ unit-mpusize: ../../include/target.h unit-mpusize.c unit-flash-erase-h7: unit-flash-erase-h7.c ../../hal/stm32h7.c gcc -o $@ unit-flash-erase-h7.c $(CFLAGS) $(LDFLAGS) +# unit-otp-keystore includes src/flash_otp_keystore.c directly (guarded to its +# host-portable code via WOLFBOOT_UNIT_TEST_OTP_KEYSTORE), so it is not a +# separate input. +unit-otp-keystore: unit-otp-keystore.c ../../src/flash_otp_keystore.c + gcc -o $@ unit-otp-keystore.c $(CFLAGS) $(LDFLAGS) + unit-update-flash-self-update: ../../include/target.h unit-update-flash.c gcc -o $@ unit-update-flash.c ../../src/image.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha256.c \ diff --git a/tools/unit-tests/unit-otp-keystore.c b/tools/unit-tests/unit-otp-keystore.c new file mode 100644 index 00000000..609a1fb7 --- /dev/null +++ b/tools/unit-tests/unit-otp-keystore.c @@ -0,0 +1,126 @@ +/* unit-otp-keystore.c + * + * Unit tests for keystore_get_size() in src/flash_otp_keystore.c. + * + * + * 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 + +/* flash_otp_keystore.c normally pulls in wolfboot/wolfboot.h and hal.h, which + * drag in target/HAL specifics. Compile it in isolation (guarded by + * WOLFBOOT_UNIT_TEST_OTP_KEYSTORE) and provide the few constants and the OTP + * read mock it needs. */ +#define WOLFBOOT_UNIT_TEST_OTP_KEYSTORE +#define FLASH_OTP_KEYSTORE +#define KEYSTORE_PUBKEY_SIZE 64 /* model an ECC-256 keystore slot */ +#define OTP_SIZE 1024 +#define FLASH_OTP_BASE 0 + +/* Mock OTP flash backing store and reader. With FLASH_OTP_BASE == 0 the address + * passed by the driver is a plain offset into this buffer. */ +static uint8_t mock_otp[OTP_SIZE]; + +int hal_flash_otp_read(uint32_t flashAddress, void *data, uint32_t length) +{ + if (flashAddress + length > (uint32_t)OTP_SIZE) + return -1; + memcpy(data, mock_otp + flashAddress, length); + return 0; +} + +#include "../../src/flash_otp_keystore.c" + +/* Provision the mock OTP with a single keystore slot whose pubkey_size field is + * set to the supplied (possibly bogus) value. */ +static void setup_otp(uint32_t pubkey_size) +{ + struct wolfBoot_otp_hdr *hdr = (struct wolfBoot_otp_hdr *)mock_otp; + struct keystore_slot *slot = + (struct keystore_slot *)(mock_otp + OTP_HDR_SIZE); + + memset(mock_otp, 0, sizeof(mock_otp)); + memcpy(hdr->keystore_hdr_magic, KEYSTORE_HDR_MAGIC, 8); + hdr->item_count = 1; + slot->slot_id = 0; + slot->key_type = 1; + slot->part_id_mask = 0xFFFFFFFF; + slot->pubkey_size = pubkey_size; +} + +/* A correctly provisioned slot returns its real size unchanged. */ +START_TEST(test_valid_size_passthrough) +{ + setup_otp(KEYSTORE_PUBKEY_SIZE); + ck_assert_int_eq(keystore_get_size(0), KEYSTORE_PUBKEY_SIZE); +} +END_TEST + +/* Regression for F-4790: keystore_get_size() must never report a size larger + * than the KEYSTORE_PUBKEY_SIZE-byte buffer the pubkey is cached in. Without the + * clamp it returned the raw OTP value (here 2*KEYSTORE_PUBKEY_SIZE), which the + * callers (key_sha256/key_sha384/key_sha3_384 and the ECC verify path) used as a + * hash length / coordinate offset, reading past otp_slot_item_cache. */ +START_TEST(test_oversize_rejected) +{ + int sz; + setup_otp(2 * KEYSTORE_PUBKEY_SIZE); + sz = keystore_get_size(0); + ck_assert_int_le(sz, KEYSTORE_PUBKEY_SIZE); + ck_assert_int_eq(sz, -1); +} +END_TEST + +/* One byte over the buffer is still out of bounds and must be rejected. */ +START_TEST(test_just_over_rejected) +{ + setup_otp(KEYSTORE_PUBKEY_SIZE + 1); + ck_assert_int_eq(keystore_get_size(0), -1); +} +END_TEST + +Suite *otp_keystore_suite(void) +{ + Suite *s = suite_create("otp-keystore"); + TCase *tc = tcase_create("otp-keystore"); + + tcase_add_test(tc, test_valid_size_passthrough); + tcase_add_test(tc, test_oversize_rejected); + tcase_add_test(tc, test_just_over_rejected); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = otp_keystore_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +}