From 682cb3b56419a242b54605cc734b99ffdcab8213 Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Thu, 2 Nov 2023 15:44:12 -0700 Subject: [PATCH] fmap: Die immediately on verification failure A recent security audit has exposed a TOCTOU risk in the FMAP verification code: if the flash returns a tampered FMAP during the first setup_preram_cache(), we will abort generating the cache but only after already filling the persistent CAR/SRAM region with the tampered version. Then we will fall back into the direct access path, which could succeed if the flash now returns the original valid FMAP. In later stages, we will just use the data from the persistent CAR/SRAM region as long as it looks like an FMAP without verifying the hash again (because the hash is only linked into the initial stage). This patch fixes the issue by just calling die() immediately if FMAP hash verification fails. When the verification fails, there's no recourse anyway -- if we're not dying here we would be dying in cbfs_get_boot_device() instead. There is no legitimate scenario where it would still be possible to continue booting after this hash verification fails. Change-Id: I59ec91c3e5a59fdd960b0ba54ae5f15ddb850480 Signed-off-by: Julius Werner Reviewed-on: https://review.coreboot.org/c/coreboot/+/78903 Reviewed-by: Yu-Ping Wu Tested-by: build bot (Jenkins) --- src/lib/fmap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/fmap.c b/src/lib/fmap.c index ed186e0cae..8d7b6a8f71 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -38,8 +38,10 @@ static int verify_fmap(const struct fmap *fmap) if (!CONFIG(CBFS_VERIFICATION) || !ENV_INITIAL_STAGE || done) return 0; /* Only need to check hash in first stage. */ + /* On error we need to die right here, lest we risk a TOCTOU attack where the cache is + filled with a tampered FMAP but the later fallback path is fed a valid one. */ if (metadata_hash_verify_fmap(fmap, FMAP_SIZE) != VB2_SUCCESS) - return -1; + die("FMAP verification failure"); done = true; return 0;