From 7164abff0b5460f5621369656a81a1fbc1fe6186 Mon Sep 17 00:00:00 2001 From: Sergii Dmytruk Date: Mon, 23 Sep 2024 21:07:55 +0300 Subject: [PATCH] drivers/efi/capsules: check for overflows of capsule sizes As was pointed out in comments on CB:83422 [0], the code lacks overflow checks: - when computing size of capsules in a single capsule block - when computing size of capsules in all capsule blocks If an overflow is triggered, the code might allocate a capsule buffer smaller than the data that's going to be written to it leading to overwriting memory after the buffer. [0]: https://review.coreboot.org/c/coreboot/+/83422 Change-Id: I43d17d77197fc2cbd721d47941101551603c352a Signed-off-by: Sergii Dmytruk Reviewed-on: https://review.coreboot.org/c/coreboot/+/84541 Reviewed-by: Krystian Hebel Reviewed-by: Paul Menzel Tested-by: build bot (Jenkins) --- src/drivers/efi/capsules.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/drivers/efi/capsules.c b/src/drivers/efi/capsules.c index e674e33228..38178c618e 100644 --- a/src/drivers/efi/capsules.c +++ b/src/drivers/efi/capsules.c @@ -344,7 +344,15 @@ static struct block_descr check_capsule_block(struct block_descr first_block, goto error; } - data_size += ALIGN_UP(capsule_hdr->CapsuleImageSize, CAPSULE_ALIGNMENT); + uint64_t capsule_size = + ALIGN_UP((uint64_t)capsule_hdr->CapsuleImageSize, CAPSULE_ALIGNMENT); + if (data_size + capsule_size < data_size) { /* overflow detection */ + printk(BIOS_ERR, + "capsules: capsules block size is too large (%#llx + %#llx) for uint64.\n", + data_size, capsule_size); + goto error; + } + data_size += capsule_size; uint32_t size_left = capsule_hdr->CapsuleImageSize; while (size_left != 0) { @@ -384,6 +392,12 @@ static struct block_descr check_capsule_block(struct block_descr first_block, } /* Increase the size only on successful parsing of the capsule block. */ + if (*total_data_size + data_size < *total_data_size) { /* overflow detection */ + printk(BIOS_ERR, + "capsules: total capsule's size is too large (%#llx + %#llx) for uint64.\n", + *total_data_size, data_size); + goto error; + } *total_data_size += data_size; return block;