[PATCH RESEND 0/6] efi_loader: capsule: check sizes against the capsule

[PATCH RESEND 0/6] efi_loader: capsule: check sizes against the capsule

[Ana Bobillo]

This is a resend of the series posted on 22 October, now with the
patches inline: the first posting carried them as attachments, which b4
and Patchwork cannot use. The patches are unchanged and still apply to
main.

The capsule update code takes several lengths and offsets from a capsule
and uses them without checking them against the size of the capsule. On
the capsule on disk path this happens before the image is authenticated,
so a capsule file in \EFI\UpdateCapsule on the ESP makes U-Boot read
outside the capsule buffer the next time capsule on disk runs, even
where CONFIG_EFI_CAPSULE_AUTHENTICATE rejects the capsule afterwards.

Patches 1-4 check the capsule file size, the capsule header size, the
FMP capsule layout (header, item offset list, item offsets, image and
vendor code sizes) and the certificate length of an image, each against
the size of the capsule. Patch 5 checks the FMP payload header size;
with CONFIG_EFI_CAPSULE_AUTHENTICATE that header is part of the signed
image, so there it is hardening. Patch 6 adds unit tests for the FMP and
certificate checks: without patches 1-5 four of them fail and the
certificate one ends in the crash described below; with them all five
pass, as does the rest of "ut lib".

The capsule file check, the FWU accept capsule and the FMP payload
header have no unit test, as they need a file system, FWU or a signed
capsule. I checked patches 1-4 on sandbox_defconfig with CONFIG_VALGRIND
instead, by placing malformed capsule files in \EFI\UpdateCapsule and
running "efidebug capsule disk-update". Without the series valgrind
reports reads past the capsule buffer in efi_capsule_read_file(),
efi_capsule_update_firmware() and efi_parse_pkcs7_header(), and a
capsule ending inside its FMP header is reported as applied; with the
series each one is rejected and none of those reads remains.

On that MbedTLS build a malformed certificate of valid length still
crashes sandbox in efi_signature_verify(), with or without this series:
pkcs7_parse_message() returns ERR_PTR() of an MbedTLS error code, which
IS_ERR() does not catch. Vsevolod Kozlov's "lib/mbedtls: Do not use
ERR_PTR with mbedTLS error values" fixes that.

Ana Bobillo (6):
  efi_loader: capsule: check the size of a capsule file
  efi_loader: capsule: check the capsule header size
  efi_loader: capsule: check the FMP capsule against its size
  efi_loader: capsule: check the certificate length of an image
  efi_loader: firmware: check the FMP payload header size
  test: lib: add tests for the capsule size checks

 lib/efi_loader/efi_capsule.c  |  63 ++++++++---
 lib/efi_loader/efi_firmware.c |  20 +++-
 test/lib/Makefile             |   1 +
 test/lib/efi_capsule.c        | 193 ++++++++++++++++++++++++++++++++++
 4 files changed, 259 insertions(+), 18 deletions(-)
 create mode 100644 test/lib/efi_capsule.c


base-commit: acc808479730e13e74199119eb9b2b9113999244

[PATCH RESEND 1/6] efi_loader: capsule: check the size of a capsule file

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_capsule_read_file() compares the size of the capsule file with the
capsule_image_size field of its header, but does not check that the
file holds a whole header first. A capsule file shorter than struct
efi_capsule_header makes it read past the end of the buffer it
allocated for the file.

Reject a capsule file shorter than its header.

Fixes: c74cd8bd08d1 ("efi_loader: capsule: add capsule_on_disk support")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_capsule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index a77810fa15c..54c118736cf 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -1190,7 +1190,7 @@ static efi_status_t efi_capsule_read_file(const u16 *filename,
 	/* fetch data */
 	ret = EFI_CALL((*fh->read)(fh, &size, buf));
 	if (ret == EFI_SUCCESS) {
-		if (size >= buf->capsule_image_size) {
+		if (size >= sizeof(*buf) && size >= buf->capsule_image_size) {
 			*capsule = buf;
 		} else {
 			free(buf);

[PATCH RESEND 2/6] efi_loader: capsule: check the capsule header size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

On the capsule on disk path, efi_capsule_update_firmware() is called
without the header checks of efi_update_capsule(). It only checks
header_size against the size of the FMP capsule header, so a
header_size smaller than struct efi_capsule_header is accepted, and the
empty capsules of the FWU multi bank update are handled before any
check at all: fwu_empty_capsule_process() reads the image GUID of an
accept capsule at header_size, which may lie anywhere after the
capsule.

Check that header_size covers the capsule header and lies within the
capsule before handling any capsule, and that an accept capsule holds
the image GUID.

Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
Fixes: 86794052418b ("FWU: Add support for the FWU Multi Bank Update feature")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_capsule.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 54c118736cf..e8835315e86 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -445,7 +445,14 @@ static __maybe_unused efi_status_t fwu_empty_capsule_process(
 		/*
 		 * Image accepted by the OS. Set the acceptance
 		 * status for the image.
+		 *
+		 * The caller has checked that header_size does not exceed
+		 * capsule_image_size.
 		 */
+		if (capsule->capsule_image_size - capsule->header_size <
+		    sizeof(*image_guid))
+			return EFI_INVALID_PARAMETER;
+
 		image_guid = (void *)(char *)capsule +
 			capsule->header_size;
 
@@ -544,6 +551,11 @@ static efi_status_t efi_capsule_update_firmware(
 	uint update_index;
 	bool fw_accept_os;
 
+	/* sanity check */
+	if (capsule_data->header_size < sizeof(*capsule_data) ||
+	    capsule_data->header_size > capsule_data->capsule_image_size)
+		return EFI_INVALID_PARAMETER;
+
 	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
 		if (fwu_empty_capsule(capsule_data)) {
 			if (fwu_empty_capsule_checks_pass()) {

[PATCH RESEND 3/6] efi_loader: capsule: check the FMP capsule against its size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_capsule_update_firmware() reads the FMP capsule header, its item
offset list and each image header without checking them against the
size of the capsule:

- the capsule may end before the end of the FMP capsule header;
- the item offset list is indexed up to embedded_driver_count +
  payload_item_count - 1, which may run past the end of the capsule;
- the check of an item offset adds the image header size to a 64-bit
  offset and can wrap around, letting an offset close to U64_MAX
  through;
- update_image_size is passed on to the FMP driver without checking
  that the image fits in the capsule.

On the capsule on disk path all of this happens before the FMP driver
authenticates the image: a capsule file in \EFI\UpdateCapsule on the
ESP makes U-Boot read outside the capsule buffer the next time capsule
on disk runs.

Check each of them against the capsule size, and check that the vendor
code fits after the image too, along the lines of EDK2's
ValidateFmpCapsule().

Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_capsule.c | 45 ++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index e8835315e86..a8d57630a6f 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -538,11 +538,13 @@ static efi_status_t efi_capsule_update_firmware(
 {
 	struct efi_firmware_management_capsule_header *capsule;
 	struct efi_firmware_management_capsule_image_header *image;
-	size_t capsule_size, image_binary_size;
+	size_t capsule_size, image_binary_size, remaining;
 	void *image_binary, *vendor_code;
 	efi_handle_t *handles;
 	efi_uintn_t no_handles;
 	int item;
+	uint item_count;
+	u64 item_offset;
 	struct efi_firmware_management_protocol *fmp;
 	u16 *abort_reason;
 	efi_guid_t *image_type_id;
@@ -588,18 +590,27 @@ static efi_status_t efi_capsule_update_firmware(
 		return EFI_UNSUPPORTED;
 	}
 
-	/* sanity check */
-	if (capsule_data->header_size < sizeof(*capsule) ||
-	    capsule_data->header_size >= capsule_data->capsule_image_size)
-		return EFI_INVALID_PARAMETER;
-
 	capsule = (void *)capsule_data + capsule_data->header_size;
 	capsule_size = capsule_data->capsule_image_size
 			- capsule_data->header_size;
 
+	/* sanity check */
+	if (capsule_size < sizeof(*capsule)) {
+		log_err("Capsule does not have enough data\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
 	if (capsule->version != 0x00000001)
 		return EFI_UNSUPPORTED;
 
+	item_count = capsule->embedded_driver_count +
+		     capsule->payload_item_count;
+	if (item_count > (capsule_size - sizeof(*capsule)) /
+			 sizeof(*capsule->item_offset_list)) {
+		log_err("Capsule does not have enough data\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
 	handles = NULL;
 	ret = EFI_CALL(efi_locate_handle_buffer(
 			BY_PROTOCOL,
@@ -609,24 +620,34 @@ static efi_status_t efi_capsule_update_firmware(
 		return EFI_UNSUPPORTED;
 
 	/* Payload */
-	for (item = capsule->embedded_driver_count;
-	     item < capsule->embedded_driver_count
-		    + capsule->payload_item_count; item++) {
+	for (item = capsule->embedded_driver_count; item < item_count;
+	     item++) {
 		/* sanity check */
-		if ((capsule->item_offset_list[item] + sizeof(*image)
-				 >= capsule_size)) {
+		item_offset = capsule->item_offset_list[item];
+		if (item_offset >= capsule_size ||
+		    capsule_size - item_offset <= sizeof(*image)) {
 			log_err("Capsule does not have enough data\n");
 			ret = EFI_INVALID_PARAMETER;
 			goto out;
 		}
 
-		image = (void *)capsule + capsule->item_offset_list[item];
+		image = (void *)capsule + item_offset;
 
 		if (image->version != 0x00000003) {
 			ret = EFI_UNSUPPORTED;
 			goto out;
 		}
 
+		/* the image and the vendor code must fit in the capsule */
+		remaining = capsule_size - item_offset - sizeof(*image);
+		if (image->update_image_size > remaining ||
+		    image->update_vendor_code_size >
+				remaining - image->update_image_size) {
+			log_err("Capsule does not have enough data\n");
+			ret = EFI_INVALID_PARAMETER;
+			goto out;
+		}
+
 		/* find a device for update firmware */
 		fmp = efi_fmp_find(&image->update_image_type_id,
 				   image->update_image_index,

[PATCH RESEND 4/6] efi_loader: capsule: check the certificate length of an image

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_remove_auth_hdr() skips the authentication header of an image by
the dwLength of its certificate without checking it against the image
size. A dwLength larger than the image makes the image size wrap
around:

- with CONFIG_EFI_CAPSULE_AUTHENTICATE, efi_capsule_authenticate()
  then parses dwLength bytes of certificate past the end of the image
  before the signature is checked;
- without it, efi_capsule_update_firmware() passes the wrapped image
  size on to the FMP driver.

Reject a certificate that does not fit in the image.

Fixes: 04be98bd6bcf ("efi: capsule: Add support for uefi capsule authentication")
Fixes: c2cc60c1f93b ("efi_loader: capsule: remove authentication data")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_capsule.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index a8d57630a6f..a3da22aa396 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -270,7 +270,9 @@ static efi_status_t efi_remove_auth_hdr(void **image, efi_uintn_t *image_size)
 		goto out;
 
 	if (auth_hdr->auth_info.hdr.dwLength <=
-	    offsetof(struct win_certificate_uefi_guid, cert_data))
+	    offsetof(struct win_certificate_uefi_guid, cert_data) ||
+	    auth_hdr->auth_info.hdr.dwLength >
+	    *image_size - sizeof(auth_hdr->monotonic_count))
 		goto out;
 
 	*image = (uint8_t *)*image + sizeof(auth_hdr->monotonic_count) +

[PATCH RESEND 5/6] efi_loader: firmware: check the FMP payload header size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_firmware_get_fw_version() reads an FMP payload header at the start
of an image without checking that the image holds one, and skips it by
its header_size without checking that against the image size. A
header_size larger than the image makes the image size wrap around
before the image is written.

Only look for the header in an image large enough to hold it, and
reject a header_size smaller than the header or larger than the image.

With CONFIG_EFI_CAPSULE_AUTHENTICATE the header is part of the signed
image, so there this is hardening.

Fixes: 675b62e12f9d ("efi_loader: Add logic to parse EDKII specific fmp payload header")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_firmware.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
index 84de95f5c23..4a5249e5d4a 100644
--- a/lib/efi_loader/efi_firmware.c
+++ b/lib/efi_loader/efi_firmware.c
@@ -499,22 +499,32 @@ efi_status_t efi_firmware_set_fmp_state_var(struct fmp_state *state, u8 image_in
  * Parse the FMP payload header and fill the fmp_state structure.
  * If no FMP payload header is found, fmp_state structure is not updated.
  *
+ * Return:		EFI_SUCCESS, or EFI_INVALID_PARAMETER for a bad header_size
  */
-static void efi_firmware_get_fw_version(const void **p_image,
-					efi_uintn_t *p_image_size,
-					struct fmp_state *state)
+static efi_status_t efi_firmware_get_fw_version(const void **p_image,
+						efi_uintn_t *p_image_size,
+						struct fmp_state *state)
 {
 	const struct fmp_payload_header *header;
 	u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE;
 
+	if (*p_image_size < sizeof(*header))
+		return EFI_SUCCESS;
+
 	header = *p_image;
 	if (header->signature == fmp_hdr_signature) {
+		if (header->header_size < sizeof(*header) ||
+		    header->header_size > *p_image_size)
+			return EFI_INVALID_PARAMETER;
+
 		/* FMP header is inserted above the capsule payload */
 		state->fw_version = header->fw_version;
 
 		*p_image += header->header_size;
 		*p_image_size -= header->header_size;
 	}
+
+	return EFI_SUCCESS;
 }
 
 /**
@@ -543,7 +553,9 @@ efi_status_t efi_firmware_verify_image(const void **p_image,
 	if (ret != EFI_SUCCESS)
 		return ret;
 
-	efi_firmware_get_fw_version(p_image, p_image_size, state);
+	ret = efi_firmware_get_fw_version(p_image, p_image_size, state);
+	if (ret != EFI_SUCCESS)
+		return ret;
 
 	image_type_id = efi_firmware_get_image_type_id(image_index);
 	if (!image_type_id)

[PATCH RESEND 6/6] test: lib: add tests for the capsule size checks

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

Add unit tests that pass malformed FMP capsules to efi_update_capsule()
and an image with a certificate longer than itself to
efi_capsule_authenticate(), and check that they are rejected:

- a capsule ending inside the FMP capsule header;
- an item offset list running past the end of the capsule;
- an item offset that wraps around when the image header size is added;
- an image or vendor code larger than the rest of the capsule;
- a certificate longer than the image.

Each capsule is built inside a larger zeroed buffer, so that without the
checks the code reads the bytes around it and returns another status
(for the certificate, a wrapped image size) instead of faulting.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 test/lib/Makefile      |   1 +
 test/lib/efi_capsule.c | 193 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+)
 create mode 100644 test/lib/efi_capsule.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 9fc4a1a9b5d..0fb6c6eabde 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -10,6 +10,7 @@ obj-y += abuf.o
 obj-y += alist.o
 obj-$(CONFIG_ECDSA_VERIFY_MBEDTLS) += ecdsa.o
 obj-$(CONFIG_EFI_LOADER) += efi_device_path.o efi_memory.o
+obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_capsule.o
 obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o
 ifdef CONFIG_RISCV
 obj-$(CONFIG_USE_PRIVATE_LIBGCC) += test_clz.o
diff --git a/test/lib/efi_capsule.c b/test/lib/efi_capsule.c
new file mode 100644
index 00000000000..7e4e1e69540
--- /dev/null
+++ b/test/lib/efi_capsule.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test the size checks of capsule updates
+ *
+ * Copyright (c) 2026 Ana Bobillo <anabobillo3@gmail.com>
+ */
+
+#include <efi_loader.h>
+#include <pe.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static const efi_guid_t fmp_capsule_guid =
+	EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
+
+/*
+ * Capsules are built in the middle of this buffer, so that the bytes
+ * before and after a capsule are zero and can be read without a fault
+ * if a size check is missing.
+ */
+static u8 buf[1024] __aligned(8);
+
+/**
+ * fmp_capsule_init() - set up an FMP capsule in the middle of buf
+ *
+ * @fmp_size:	size of the FMP capsule following the capsule header
+ * Return:	capsule header
+ */
+static struct efi_capsule_header *fmp_capsule_init(size_t fmp_size)
+{
+	struct efi_capsule_header *capsule = (void *)buf + 256;
+
+	memset(buf, 0, sizeof(buf));
+	guidcpy(&capsule->capsule_guid, &fmp_capsule_guid);
+	capsule->header_size = sizeof(*capsule);
+	capsule->capsule_image_size = sizeof(*capsule) + fmp_size;
+
+	return capsule;
+}
+
+/**
+ * update_capsule() - pass one capsule to UpdateCapsule()
+ *
+ * @capsule:	capsule header
+ * Return:	status code
+ */
+static efi_status_t update_capsule(struct efi_capsule_header *capsule)
+{
+	return EFI_CALL(efi_update_capsule(&capsule, 1, 0));
+}
+
+/* efi_capsule_update_firmware() must check the FMP capsule header size */
+static int lib_test_efi_capsule_update_fmp_header(struct unit_test_state *uts)
+{
+	struct efi_capsule_header *capsule;
+	struct efi_firmware_management_capsule_header *fmp;
+
+	/* The FWU checks reject a capsule before it is parsed */
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	/* The capsule ends after the version field */
+	capsule = fmp_capsule_init(sizeof(fmp->version));
+	fmp = (void *)capsule + capsule->header_size;
+	fmp->version = 1;
+	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_capsule_update_fmp_header, 0);
+
+/* efi_capsule_update_firmware() must check the item offset list size */
+static int lib_test_efi_capsule_update_item_list(struct unit_test_state *uts)
+{
+	struct efi_capsule_header *capsule;
+	struct efi_firmware_management_capsule_header *fmp;
+
+	/* The FWU checks reject a capsule before it is parsed */
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	/* Room for eight item offsets, but nine items */
+	capsule = fmp_capsule_init(sizeof(*fmp) + 8 * sizeof(u64));
+	fmp = (void *)capsule + capsule->header_size;
+	fmp->version = 1;
+	fmp->embedded_driver_count = 8;
+	fmp->payload_item_count = 1;
+	/* The ninth item offset lies past the end of the capsule */
+	fmp->item_offset_list[8] = sizeof(*fmp);
+	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_capsule_update_item_list, 0);
+
+/* efi_capsule_update_firmware() must check an item offset without overflow */
+static int lib_test_efi_capsule_update_item_offset(struct unit_test_state *uts)
+{
+	struct efi_capsule_header *capsule;
+	struct efi_firmware_management_capsule_header *fmp;
+	struct efi_firmware_management_capsule_image_header *image;
+
+	/* The FWU checks reject a capsule before it is parsed */
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	capsule = fmp_capsule_init(sizeof(*fmp) + sizeof(u64) +
+				   sizeof(*image) + 16);
+	fmp = (void *)capsule + capsule->header_size;
+	fmp->version = 1;
+	fmp->payload_item_count = 1;
+	/* Offset plus image header size wraps around to zero */
+	fmp->item_offset_list[0] = -(u64)sizeof(*image);
+	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_capsule_update_item_offset, 0);
+
+/* efi_capsule_update_firmware() must check the image and vendor code sizes */
+static int lib_test_efi_capsule_update_image_size(struct unit_test_state *uts)
+{
+	struct efi_capsule_header *capsule;
+	struct efi_firmware_management_capsule_header *fmp;
+	struct efi_firmware_management_capsule_image_header *image;
+
+	/* The FWU checks reject a capsule before it is parsed */
+	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	capsule = fmp_capsule_init(sizeof(*fmp) + sizeof(u64) +
+				   sizeof(*image) + 16);
+	fmp = (void *)capsule + capsule->header_size;
+	fmp->version = 1;
+	fmp->payload_item_count = 1;
+	fmp->item_offset_list[0] = sizeof(*fmp) + sizeof(u64);
+	image = (void *)fmp + fmp->item_offset_list[0];
+	image->version = 3;
+
+	/* The image is larger than the capsule */
+	image->update_image_size = 17;
+	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
+
+	/* The vendor code does not fit after the image */
+	image->update_image_size = 16;
+	image->update_vendor_code_size = 1;
+	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
+
+	/* Both fit: there is no FMP driver for the zero image type */
+	image->update_vendor_code_size = 0;
+	ut_asserteq_64(EFI_UNSUPPORTED, update_capsule(capsule));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_capsule_update_image_size, 0);
+
+/* efi_remove_auth_hdr() must check the certificate length */
+static int lib_test_efi_remove_auth_hdr_length(struct unit_test_state *uts)
+{
+	struct efi_firmware_image_authentication *auth;
+	efi_uintn_t capsule_size = 64;
+	efi_uintn_t image_size;
+	void *image;
+
+	if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
+		return -EAGAIN;
+
+	memset(buf, 0, sizeof(buf));
+	auth = (void *)buf;
+	/* The certificate is longer than the capsule */
+	auth->auth_info.hdr.dwLength = 256;
+	auth->auth_info.hdr.wRevision = WIN_CERT_REVISION_2_0;
+	auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
+	guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7);
+
+	ut_asserteq_64(EFI_SECURITY_VIOLATION,
+		       efi_capsule_authenticate(buf, capsule_size, &image,
+						&image_size));
+	/* The image size must not have wrapped around */
+	ut_asserteq_64(capsule_size, image_size);
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_remove_auth_hdr_length, 0);

Re: [PATCH RESEND 1/6] efi_loader: capsule: check the size of a capsule file

[Ilias Apalodimas]

On Mon, 26 Oct 2026 at 15:19, Ana Bobillo <anabobillo3@gmail.com> wrote:
>
> From: Ana Bobillo <anabobillo3@gmail.com>
>
> efi_capsule_read_file() compares the size of the capsule file with the
> capsule_image_size field of its header, but does not check that the
> file holds a whole header first. A capsule file shorter than struct
> efi_capsule_header makes it read past the end of the buffer it
> allocated for the file.
>
> Reject a capsule file shorter than its header.
>
> Fixes: c74cd8bd08d1 ("efi_loader: capsule: add capsule_on_disk support")
> Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
> ---

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

>  lib/efi_loader/efi_capsule.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index a77810fa15c..54c118736cf 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -1190,7 +1190,7 @@ static efi_status_t efi_capsule_read_file(const u16 *filename,
>         /* fetch data */
>         ret = EFI_CALL((*fh->read)(fh, &size, buf));
>         if (ret == EFI_SUCCESS) {
> -               if (size >= buf->capsule_image_size) {
> +               if (size >= sizeof(*buf) && size >= buf->capsule_image_size) {
>                         *capsule = buf;
>                 } else {
>                         free(buf);

Re: [PATCH RESEND 3/6] efi_loader: capsule: check the FMP capsule against its size

[Heinrich Schuchardt]

On 10/26/26 14:19, Ana Bobillo wrote:
> From: Ana Bobillo <anabobillo3@gmail.com>
> 
> efi_capsule_update_firmware() reads the FMP capsule header, its item
> offset list and each image header without checking them against the
> size of the capsule:
> 
> - the capsule may end before the end of the FMP capsule header;
> - the item offset list is indexed up to embedded_driver_count +
>    payload_item_count - 1, which may run past the end of the capsule;
> - the check of an item offset adds the image header size to a 64-bit
>    offset and can wrap around, letting an offset close to U64_MAX
>    through;
> - update_image_size is passed on to the FMP driver without checking
>    that the image fits in the capsule.
> 
> On the capsule on disk path all of this happens before the FMP driver
> authenticates the image: a capsule file in \EFI\UpdateCapsule on the
> ESP makes U-Boot read outside the capsule buffer the next time capsule
> on disk runs.
> 
> Check each of them against the capsule size, and check that the vendor
> code fits after the image too, along the lines of EDK2's
> ValidateFmpCapsule().
> 
> Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
> Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
> ---
>   lib/efi_loader/efi_capsule.c | 45 ++++++++++++++++++++++++++----------
>   1 file changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index e8835315e86..a8d57630a6f 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -538,11 +538,13 @@ static efi_status_t efi_capsule_update_firmware(
>   {
>   	struct efi_firmware_management_capsule_header *capsule;
>   	struct efi_firmware_management_capsule_image_header *image;
> -	size_t capsule_size, image_binary_size;
> +	size_t capsule_size, image_binary_size, remaining;
>   	void *image_binary, *vendor_code;
>   	efi_handle_t *handles;
>   	efi_uintn_t no_handles;
>   	int item;
> +	uint item_count;
> +	u64 item_offset;

Nits:

item and item_count should have the same signedness as you are comparing 
the two below. I think both can be unsigned.

Generally I would use size_t for indexes.

Best regards

Heinrich

>   	struct efi_firmware_management_protocol *fmp;
>   	u16 *abort_reason;
>   	efi_guid_t *image_type_id;
> @@ -588,18 +590,27 @@ static efi_status_t efi_capsule_update_firmware(
>   		return EFI_UNSUPPORTED;
>   	}
>   
> -	/* sanity check */
> -	if (capsule_data->header_size < sizeof(*capsule) ||
> -	    capsule_data->header_size >= capsule_data->capsule_image_size)
> -		return EFI_INVALID_PARAMETER;
> -
>   	capsule = (void *)capsule_data + capsule_data->header_size;
>   	capsule_size = capsule_data->capsule_image_size
>   			- capsule_data->header_size;
>   
> +	/* sanity check */
> +	if (capsule_size < sizeof(*capsule)) {
> +		log_err("Capsule does not have enough data\n");
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
>   	if (capsule->version != 0x00000001)
>   		return EFI_UNSUPPORTED;
>   
> +	item_count = capsule->embedded_driver_count +
> +		     capsule->payload_item_count;
> +	if (item_count > (capsule_size - sizeof(*capsule)) /
> +			 sizeof(*capsule->item_offset_list)) {
> +		log_err("Capsule does not have enough data\n");
> +		return EFI_INVALID_PARAMETER;
> +	}
> +
>   	handles = NULL;
>   	ret = EFI_CALL(efi_locate_handle_buffer(
>   			BY_PROTOCOL,
> @@ -609,24 +620,34 @@ static efi_status_t efi_capsule_update_firmware(
>   		return EFI_UNSUPPORTED;
>   
>   	/* Payload */
> -	for (item = capsule->embedded_driver_count;
> -	     item < capsule->embedded_driver_count
> -		    + capsule->payload_item_count; item++) {
> +	for (item = capsule->embedded_driver_count; item < item_count;
> +	     item++) {
>   		/* sanity check */
> -		if ((capsule->item_offset_list[item] + sizeof(*image)
> -				 >= capsule_size)) {
> +		item_offset = capsule->item_offset_list[item];
> +		if (item_offset >= capsule_size ||
> +		    capsule_size - item_offset <= sizeof(*image)) {
>   			log_err("Capsule does not have enough data\n");
>   			ret = EFI_INVALID_PARAMETER;
>   			goto out;
>   		}
>   
> -		image = (void *)capsule + capsule->item_offset_list[item];
> +		image = (void *)capsule + item_offset;
>   
>   		if (image->version != 0x00000003) {
>   			ret = EFI_UNSUPPORTED;
>   			goto out;
>   		}
>   
> +		/* the image and the vendor code must fit in the capsule */
> +		remaining = capsule_size - item_offset - sizeof(*image);
> +		if (image->update_image_size > remaining ||
> +		    image->update_vendor_code_size >
> +				remaining - image->update_image_size) {
> +			log_err("Capsule does not have enough data\n");
> +			ret = EFI_INVALID_PARAMETER;
> +			goto out;
> +		}
> +
>   		/* find a device for update firmware */
>   		fmp = efi_fmp_find(&image->update_image_type_id,
>   				   image->update_image_index,

Re: [PATCH RESEND 6/6] test: lib: add tests for the capsule size checks

[Heinrich Schuchardt]

On 10/26/26 14:19, Ana Bobillo wrote:
> From: Ana Bobillo <anabobillo3@gmail.com>
> 
> Add unit tests that pass malformed FMP capsules to efi_update_capsule()
> and an image with a certificate longer than itself to
> efi_capsule_authenticate(), and check that they are rejected:
> 
> - a capsule ending inside the FMP capsule header;
> - an item offset list running past the end of the capsule;
> - an item offset that wraps around when the image header size is added;
> - an image or vendor code larger than the rest of the capsule;
> - a certificate longer than the image.
> 
> Each capsule is built inside a larger zeroed buffer, so that without the
> checks the code reads the bytes around it and returns another status
> (for the certificate, a wrapped image size) instead of faulting.
> 
> Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
> ---
>   test/lib/Makefile      |   1 +
>   test/lib/efi_capsule.c | 193 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 194 insertions(+)
>   create mode 100644 test/lib/efi_capsule.c
> 
> diff --git a/test/lib/Makefile b/test/lib/Makefile
> index 9fc4a1a9b5d..0fb6c6eabde 100644
> --- a/test/lib/Makefile
> +++ b/test/lib/Makefile
> @@ -10,6 +10,7 @@ obj-y += abuf.o
>   obj-y += alist.o
>   obj-$(CONFIG_ECDSA_VERIFY_MBEDTLS) += ecdsa.o
>   obj-$(CONFIG_EFI_LOADER) += efi_device_path.o efi_memory.o
> +obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_capsule.o

Thanks for adding tests. Maybe consider if patches 1, 2, 5 can also be 
covered.

Best regards

Heinrich

>   obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o
>   ifdef CONFIG_RISCV
>   obj-$(CONFIG_USE_PRIVATE_LIBGCC) += test_clz.o
> diff --git a/test/lib/efi_capsule.c b/test/lib/efi_capsule.c
> new file mode 100644
> index 00000000000..7e4e1e69540
> --- /dev/null
> +++ b/test/lib/efi_capsule.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Test the size checks of capsule updates
> + *
> + * Copyright (c) 2026 Ana Bobillo <anabobillo3@gmail.com>
> + */
> +
> +#include <efi_loader.h>
> +#include <pe.h>
> +#include <test/lib.h>
> +#include <test/test.h>
> +#include <test/ut.h>
> +
> +static const efi_guid_t fmp_capsule_guid =
> +	EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> +
> +/*
> + * Capsules are built in the middle of this buffer, so that the bytes
> + * before and after a capsule are zero and can be read without a fault
> + * if a size check is missing.
> + */
> +static u8 buf[1024] __aligned(8);
> +
> +/**
> + * fmp_capsule_init() - set up an FMP capsule in the middle of buf
> + *
> + * @fmp_size:	size of the FMP capsule following the capsule header
> + * Return:	capsule header
> + */
> +static struct efi_capsule_header *fmp_capsule_init(size_t fmp_size)
> +{
> +	struct efi_capsule_header *capsule = (void *)buf + 256;
> +
> +	memset(buf, 0, sizeof(buf));
> +	guidcpy(&capsule->capsule_guid, &fmp_capsule_guid);
> +	capsule->header_size = sizeof(*capsule);
> +	capsule->capsule_image_size = sizeof(*capsule) + fmp_size;
> +
> +	return capsule;
> +}
> +
> +/**
> + * update_capsule() - pass one capsule to UpdateCapsule()
> + *
> + * @capsule:	capsule header
> + * Return:	status code
> + */
> +static efi_status_t update_capsule(struct efi_capsule_header *capsule)
> +{
> +	return EFI_CALL(efi_update_capsule(&capsule, 1, 0));
> +}
> +
> +/* efi_capsule_update_firmware() must check the FMP capsule header size */
> +static int lib_test_efi_capsule_update_fmp_header(struct unit_test_state *uts)
> +{
> +	struct efi_capsule_header *capsule;
> +	struct efi_firmware_management_capsule_header *fmp;
> +
> +	/* The FWU checks reject a capsule before it is parsed */
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
> +		return -EAGAIN;
> +
> +	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
> +
> +	/* The capsule ends after the version field */
> +	capsule = fmp_capsule_init(sizeof(fmp->version));
> +	fmp = (void *)capsule + capsule->header_size;
> +	fmp->version = 1;
> +	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
> +
> +	return 0;
> +}
> +LIB_TEST(lib_test_efi_capsule_update_fmp_header, 0);
> +
> +/* efi_capsule_update_firmware() must check the item offset list size */
> +static int lib_test_efi_capsule_update_item_list(struct unit_test_state *uts)
> +{
> +	struct efi_capsule_header *capsule;
> +	struct efi_firmware_management_capsule_header *fmp;
> +
> +	/* The FWU checks reject a capsule before it is parsed */
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
> +		return -EAGAIN;
> +
> +	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
> +
> +	/* Room for eight item offsets, but nine items */
> +	capsule = fmp_capsule_init(sizeof(*fmp) + 8 * sizeof(u64));
> +	fmp = (void *)capsule + capsule->header_size;
> +	fmp->version = 1;
> +	fmp->embedded_driver_count = 8;
> +	fmp->payload_item_count = 1;
> +	/* The ninth item offset lies past the end of the capsule */
> +	fmp->item_offset_list[8] = sizeof(*fmp);
> +	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
> +
> +	return 0;
> +}
> +LIB_TEST(lib_test_efi_capsule_update_item_list, 0);
> +
> +/* efi_capsule_update_firmware() must check an item offset without overflow */
> +static int lib_test_efi_capsule_update_item_offset(struct unit_test_state *uts)
> +{
> +	struct efi_capsule_header *capsule;
> +	struct efi_firmware_management_capsule_header *fmp;
> +	struct efi_firmware_management_capsule_image_header *image;
> +
> +	/* The FWU checks reject a capsule before it is parsed */
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
> +		return -EAGAIN;
> +
> +	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
> +
> +	capsule = fmp_capsule_init(sizeof(*fmp) + sizeof(u64) +
> +				   sizeof(*image) + 16);
> +	fmp = (void *)capsule + capsule->header_size;
> +	fmp->version = 1;
> +	fmp->payload_item_count = 1;
> +	/* Offset plus image header size wraps around to zero */
> +	fmp->item_offset_list[0] = -(u64)sizeof(*image);
> +	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
> +
> +	return 0;
> +}
> +LIB_TEST(lib_test_efi_capsule_update_item_offset, 0);
> +
> +/* efi_capsule_update_firmware() must check the image and vendor code sizes */
> +static int lib_test_efi_capsule_update_image_size(struct unit_test_state *uts)
> +{
> +	struct efi_capsule_header *capsule;
> +	struct efi_firmware_management_capsule_header *fmp;
> +	struct efi_firmware_management_capsule_image_header *image;
> +
> +	/* The FWU checks reject a capsule before it is parsed */
> +	if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
> +		return -EAGAIN;
> +
> +	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
> +
> +	capsule = fmp_capsule_init(sizeof(*fmp) + sizeof(u64) +
> +				   sizeof(*image) + 16);
> +	fmp = (void *)capsule + capsule->header_size;
> +	fmp->version = 1;
> +	fmp->payload_item_count = 1;
> +	fmp->item_offset_list[0] = sizeof(*fmp) + sizeof(u64);
> +	image = (void *)fmp + fmp->item_offset_list[0];
> +	image->version = 3;
> +
> +	/* The image is larger than the capsule */
> +	image->update_image_size = 17;
> +	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
> +
> +	/* The vendor code does not fit after the image */
> +	image->update_image_size = 16;
> +	image->update_vendor_code_size = 1;
> +	ut_asserteq_64(EFI_INVALID_PARAMETER, update_capsule(capsule));
> +
> +	/* Both fit: there is no FMP driver for the zero image type */
> +	image->update_vendor_code_size = 0;
> +	ut_asserteq_64(EFI_UNSUPPORTED, update_capsule(capsule));
> +
> +	return 0;
> +}
> +LIB_TEST(lib_test_efi_capsule_update_image_size, 0);
> +
> +/* efi_remove_auth_hdr() must check the certificate length */
> +static int lib_test_efi_remove_auth_hdr_length(struct unit_test_state *uts)
> +{
> +	struct efi_firmware_image_authentication *auth;
> +	efi_uintn_t capsule_size = 64;
> +	efi_uintn_t image_size;
> +	void *image;
> +
> +	if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE))
> +		return -EAGAIN;
> +
> +	memset(buf, 0, sizeof(buf));
> +	auth = (void *)buf;
> +	/* The certificate is longer than the capsule */
> +	auth->auth_info.hdr.dwLength = 256;
> +	auth->auth_info.hdr.wRevision = WIN_CERT_REVISION_2_0;
> +	auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
> +	guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7);
> +
> +	ut_asserteq_64(EFI_SECURITY_VIOLATION,
> +		       efi_capsule_authenticate(buf, capsule_size, &image,
> +						&image_size));
> +	/* The image size must not have wrapped around */
> +	ut_asserteq_64(capsule_size, image_size);
> +
> +	return 0;
> +}
> +LIB_TEST(lib_test_efi_remove_auth_hdr_length, 0);