[PATCH v3 00/11] efi_loader: check PE images before using them

[PATCH v3 00/11] efi_loader: check PE images before using them

[Ana Bobillo]

U-Boot takes the offsets and sizes of a PE/COFF image from its headers
and uses them before it knows whether the image may run. With Secure
Boot on, efi_load_pe() authenticates the image first but goes on
copying and relocating it when authentication fails, and only then
returns EFI_SECURITY_VIOLATION; efi_image_parse(), which collects the
regions to hash, runs before the signature is checked, and so does the
walk of the certificate table. TCG2 measured boot parses every image as
well. So an image that will be refused, or one that is only measured,
can make U-Boot read or write outside its buffers, or hang.

This series checks those fields against the image where they are read,
along the lines of EDK2's PeCoffLoaderLib and DxeImageVerificationLib:

- patch 1 has efi_check_pe() check for a 64bit NT header again, as its
  comment says;
- patch 2 checks the headers, the sections and the certificate table in
  efi_image_parse(), and fails on errors of efi_image_region_add();
- patches 3 and 4 have efi_load_pe() reject, before allocating the
  image, a section whose end wraps around and section data past the end
  of the file; patch 5 makes room in the loaded image for all of the
  headers;
- patch 6 checks the relocation table against the image;
- patch 7 rejects a certificate table holding a certificate no longer
  than its header, which could make the walk loop forever, or longer
  than the rest of the table;
- patch 8 refuses to load an image whose subsystem is not one of the
  UEFI image types, as EDK2 does;
- patch 9 marks an image whose TCG2 measurement failed as not
  authenticated, so that it is not started unmeasured;
- patches 10 and 11 add unit tests and a Secure Boot test.

Patch 9 no longer needs Heinrich's "efi_loader: handling of missing TPM
in efi_load_pe()" [1]: the series applies to main with or without [1],
and [1] applies on top of the series.

barebox, whose EFI loader is based on this file, made several of the
same changes this year; patches 2, 3 and 4 follow them and say so.

Valid images are unaffected: the unit tests check PE32 and PE32+ images
with 16, 6 or 5 data directories, an unpadded last section and a file
alignment of 0, the EFI selftests and the Secure Boot tests pass, and
each patch builds on its own. Tested on sandbox.

v2 passed U-Boot's Azure CI, all jobs, with [1] (build 13728); v3
changes only patches 7 and 9.

[1] https://patchwork.ozlabs.org/patch/2335140/

Changes in v3:
- rebase on current main; patch 9 no longer needs [1] (Ilias)
- patch 7: fold the new check and the old "dwLength too small" one into
  a single check, which rejects a certificate no longer than its header
  or longer than the rest of the table (Ilias)
- patch 9: put the new check ahead of the one for a missing TPM, so
  that the patch applies with or without [1]; the commit message says
  that an image is refused when no TPM is found, which holds with or
  without [1]
- add Ilias's Reviewed-by to patch 8

Changes in v2:

Ana Bobillo (11):
  efi_loader: check the NT header size in efi_check_pe()
  efi_loader: check the PE image ranges in efi_image_parse()
  efi_loader: reject a PE section whose end wraps around
  efi_loader: check PE section raw data against the image size
  efi_loader: make room for the PE headers in the loaded image
  efi_loader: check the relocation table against the image size
  efi_loader: check certificate lengths in efi_image_authenticate()
  efi_loader: refuse a PE image of an unknown subsystem
  efi_loader: do not start an image that cannot be measured
  test: lib: add tests for the PE image checks
  test: py: test invalid certificate lengths in signed images

 lib/efi_loader/efi_image_loader.c             | 369 +++++++++----
 test/lib/Makefile                             |   2 +-
 test/lib/efi_image_loader.c                   | 501 ++++++++++++++++++
 test/py/tests/test_efi_secboot/conftest.py    |  31 ++
 test/py/tests/test_efi_secboot/test_signed.py |  36 ++
 5 files changed, 833 insertions(+), 106 deletions(-)
 create mode 100644 test/lib/efi_image_loader.c


base-commit: feb8913982e4462c01fd914271f92254a25dc101

[PATCH v3 01/11] efi_loader: check the NT header size in efi_check_pe()

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_check_pe() is meant to check that the image holds a 64bit NT header,
which is longer than a 32bit one, as its comment says. Since commit
5dad05a0e61d ("efi_loader: carve out efi_check_pe()") it checks for a
32bit one only, so the callers read the optional header fields and data
directories of a PE32+ image past the end of a truncated image. The
addition of e_lfanew can also wrap around on 32bit systems.

Check for the size of the 64bit NT header again, without wrapping, and
declare the header pointer with that type too.

Fixes: 5dad05a0e61d ("efi_loader: carve out efi_check_pe()")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_image_loader.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index f9a2d2df405..7cda338882c 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -835,7 +835,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
 efi_status_t efi_check_pe(void *buffer, size_t size, void **nt_header)
 {
 	IMAGE_DOS_HEADER *dos = buffer;
-	IMAGE_NT_HEADERS32 *nt;
+	IMAGE_NT_HEADERS64 *nt;
 
 	if (size < sizeof(*dos))
 		return EFI_INVALID_PARAMETER;
@@ -849,9 +849,10 @@ efi_status_t efi_check_pe(void *buffer, size_t size, void **nt_header)
 	 * least one section header follows we only need to check for the length
 	 * of the 64bit header which is longer than the 32bit header.
 	 */
-	if (size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32))
+	if (dos->e_lfanew > size ||
+	    size - dos->e_lfanew < sizeof(*nt))
 		return EFI_INVALID_PARAMETER;
-	nt = (IMAGE_NT_HEADERS32 *)((u8 *)buffer + dos->e_lfanew);
+	nt = (IMAGE_NT_HEADERS64 *)((u8 *)buffer + dos->e_lfanew);
 
 	/* Check for PE-COFF magic */
 	if (nt->Signature != IMAGE_NT_SIGNATURE)

[PATCH v3 02/11] efi_loader: check the PE image ranges in efi_image_parse()

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_image_parse() collects the regions of an image to be hashed for
Secure Boot and for TCG2 measurements. It takes the header size, the
section table, each section's raw data and the certificate table from
the image without checking them against the image size:

- SizeOfHeaders and each section's PointerToRawData and SizeOfRawData
  are added as regions even when they run past the end of the image,
  which is then read out of bounds when the regions are hashed;
- the section table is read wherever SizeOfOptionalHeader and
  NumberOfSections put it;
- the end of the certificate table is computed in 32 bits, so a large
  offset wraps around and passes the check;
- errors of efi_image_region_add() are ignored.

All of this happens before the signature is verified.

Call efi_check_pe() first, check each offset and size against the image
size without wrapping, add regions only through a checked helper and fail
on its errors. Require the optional header to hold exactly the data
directories it declares, at most 16, as EDK2 and shim do. Take a file
alignment of 0 as the PECOFF default of 512 and require any other to be
a power of two. The padding of a section to the file alignment is hashed
only as far as the image goes, so an image whose last section is not
padded is still accepted. Check the certificate table against the size
of WIN_CERTIFICATE, not of a pointer to it. The sorted section table is
now freed on the error path too, which the new error exits from the
section loop need.

barebox, whose EFI loader is based on this file, added similar checks in
4ec1f7166778 ("efi: validate PE parser input ranges").

Fixes: 4540dabdcaca ("efi_loader: image_loader: support image authentication")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_image_loader.c | 254 +++++++++++++++++++++---------
 1 file changed, 176 insertions(+), 78 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 7cda338882c..82083cb73d9 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -393,6 +393,45 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
 	}
 }
 
+/**
+ * pe_range_ok() - check that a range lies within the image
+ * @len:	size of the image
+ * @off:	offset of the range from the start of the image
+ * @size:	size of the range
+ *
+ * Return:	true if [@off, @off + @size) lies within [0, @len)
+ */
+static bool pe_range_ok(size_t len, size_t off, size_t size)
+{
+	return off <= len && size <= len - off;
+}
+
+/**
+ * efi_image_region_add_checked() - add a region of an image
+ * @regs:	pointer to array of regions
+ * @efi:	pointer to the image
+ * @len:	size of the image
+ * @start:	offset of the region in the image
+ * @size:	size of the region
+ * @nocheck:	flag against overlapped regions
+ *
+ * Add the region [@start, @start + @size[ of the image to @regs if it lies
+ * within the image.
+ *
+ * Return:	status code
+ */
+static efi_status_t efi_image_region_add_checked(struct efi_image_regions *regs,
+						 const void *efi, size_t len,
+						 size_t start, size_t size,
+						 int nocheck)
+{
+	if (!pe_range_ok(len, start, size))
+		return EFI_INVALID_PARAMETER;
+
+	return efi_image_region_add(regs, efi + start, efi + start + size,
+				    nocheck);
+}
+
 /**
  * efi_image_parse() - parse a PE image
  * @efi:	Pointer to image
@@ -401,8 +440,8 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
  * @auth:	Pointer to a pointer to authentication data in PE
  * @auth_len:	Size of @auth
  *
- * Parse image binary in PE32(+) format, assuming that sanity of PE image
- * has been checked by a caller.
+ * Parse image binary in PE32(+) format. Each offset and size taken from the
+ * image is checked against @len before it is used.
  * On success, an address of authentication data in @efi and its size will
  * be returned in @auth and @auth_len, respectively.
  *
@@ -411,97 +450,142 @@ void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
 bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 		     WIN_CERTIFICATE **auth, size_t *auth_len)
 {
-	struct efi_image_regions *regs;
-	IMAGE_DOS_HEADER *dos;
+	struct efi_image_regions *regs = NULL;
 	IMAGE_NT_HEADERS32 *nt;
-	IMAGE_SECTION_HEADER *sections, **sorted;
+	IMAGE_SECTION_HEADER *sections, **sorted = NULL;
 	int num_regions, num_sections, i;
 	int ctidx = IMAGE_DIRECTORY_ENTRY_SECURITY;
-	u32 align, size, authsz, authoff;
-	size_t bytes_hashed;
+	u32 align, authsz, authoff, header_size, num_dirs;
+	size_t bytes_hashed, opt_off, opt_size, dirs_start;
+	size_t csum_off, subsys_off, dir_off, dir_end;
+	size_t sections_off, sections_size;
 
-	dos = (void *)efi;
-	nt = (void *)(efi + dos->e_lfanew);
 	authoff = 0;
 	authsz = 0;
 
 	/*
-	 * Count maximum number of regions to be digested.
-	 * We don't have to have an exact number here.
-	 * See efi_image_region_add()'s in parsing below.
+	 * efi_check_pe() checks the DOS and PE signatures and that the image
+	 * is large enough to hold a 64bit NT header, which is longer than a
+	 * 32bit one. Every optional header field and data directory entry
+	 * read below therefore lies within the image.
 	 */
-	num_regions = 3; /* for header */
-	num_regions += nt->FileHeader.NumberOfSections;
-	num_regions++; /* for extra */
-
-	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
-		      1);
-	if (!regs)
-		goto err;
-	regs->max = num_regions;
+	if (efi_check_pe(efi, len, (void **)&nt) != EFI_SUCCESS)
+		return false;
+	opt_off = (u8 *)&nt->OptionalHeader - (u8 *)efi;
+	opt_size = nt->FileHeader.SizeOfOptionalHeader;
 
-	/*
-	 * Collect data regions for hash calculation
-	 * 1. File headers
-	 */
 	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
 		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
 		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
 
-		/* Skip CheckSum */
-		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
-		if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
-			efi_image_region_add(regs,
-					     &opt->Subsystem,
-					     efi + opt->SizeOfHeaders, 0);
-		} else {
-			/* Skip Certificates Table */
-			efi_image_region_add(regs,
-					     &opt->Subsystem,
-					     &opt->DataDirectory[ctidx], 0);
-			efi_image_region_add(regs,
-					     &opt->DataDirectory[ctidx] + 1,
-					     efi + opt->SizeOfHeaders, 0);
-
+		dirs_start = offsetof(IMAGE_OPTIONAL_HEADER64, DataDirectory);
+		csum_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+					      CheckSum);
+		subsys_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+						Subsystem);
+		dir_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER64,
+					     DataDirectory[ctidx]);
+		header_size = opt->SizeOfHeaders;
+		align = opt->FileAlignment;
+		num_dirs = opt->NumberOfRvaAndSizes;
+		if (num_dirs > ctidx) {
 			authoff = opt->DataDirectory[ctidx].VirtualAddress;
 			authsz = opt->DataDirectory[ctidx].Size;
 		}
-
-		bytes_hashed = opt->SizeOfHeaders;
-		align = opt->FileAlignment;
 	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
 
-		/* Skip CheckSum */
-		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
-		if (nt->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
-			efi_image_region_add(regs,
-					     &opt->Subsystem,
-					     efi + opt->SizeOfHeaders, 0);
-		} else {
-			/* Skip Certificates Table */
-			efi_image_region_add(regs, &opt->Subsystem,
-					     &opt->DataDirectory[ctidx], 0);
-			efi_image_region_add(regs,
-					     &opt->DataDirectory[ctidx] + 1,
-					     efi + opt->SizeOfHeaders, 0);
-
+		dirs_start = offsetof(IMAGE_OPTIONAL_HEADER32, DataDirectory);
+		csum_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+					      CheckSum);
+		subsys_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+						Subsystem);
+		dir_off = opt_off + offsetof(IMAGE_OPTIONAL_HEADER32,
+					     DataDirectory[ctidx]);
+		header_size = opt->SizeOfHeaders;
+		align = opt->FileAlignment;
+		num_dirs = opt->NumberOfRvaAndSizes;
+		if (num_dirs > ctidx) {
 			authoff = opt->DataDirectory[ctidx].VirtualAddress;
 			authsz = opt->DataDirectory[ctidx].Size;
 		}
-
-		bytes_hashed = opt->SizeOfHeaders;
-		align = opt->FileAlignment;
 	} else {
 		log_err("%s: Invalid optional header magic %x\n", __func__,
 			nt->OptionalHeader.Magic);
+		return false;
+	}
+	dir_end = dir_off + sizeof(IMAGE_DATA_DIRECTORY);
+
+	/*
+	 * The optional header must hold exactly the data directories it
+	 * declares, at most 16, as EDK2 and shim require. The section table
+	 * follows the optional header; it and the whole header area
+	 * (SizeOfHeaders) must lie within the image, as must the certificate
+	 * table.
+	 */
+	num_sections = nt->FileHeader.NumberOfSections;
+	sections_off = opt_off + opt_size;
+	sections_size = (size_t)num_sections * sizeof(IMAGE_SECTION_HEADER);
+	if (num_dirs > IMAGE_NUMBEROF_DIRECTORY_ENTRIES ||
+	    opt_size != dirs_start + num_dirs * sizeof(IMAGE_DATA_DIRECTORY) ||
+	    !pe_range_ok(len, sections_off, sections_size) ||
+	    !pe_range_ok(len, 0, header_size) ||
+	    header_size < sections_off + sections_size) {
+		log_err("%s: Invalid PE headers\n", __func__);
+		return false;
+	}
+	if (authsz && !pe_range_ok(len, authoff, authsz)) {
+		log_err("%s: Size for auth too large: %u\n", __func__, authsz);
+		return false;
+	}
+
+	/* The file alignment is a power of two, by default 512 (PECOFF) */
+	if (!align)
+		align = 512;
+	if (align & (align - 1)) {
+		log_err("%s: Invalid file alignment %u\n", __func__, align);
+		return false;
+	}
+
+	/*
+	 * Count maximum number of regions to be digested: three for the
+	 * header, one for each section and one for extra data.
+	 * We don't have to have an exact number here.
+	 * See efi_image_region_add()'s in parsing below.
+	 */
+	num_regions = 4 + num_sections;
+
+	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
+		      1);
+	if (!regs)
 		goto err;
+	regs->max = num_regions;
+
+	/*
+	 * Collect data regions for hash calculation
+	 * 1. File headers
+	 */
+	/* Skip CheckSum */
+	if (efi_image_region_add_checked(regs, efi, len, 0, csum_off, 0))
+		goto err;
+	if (num_dirs <= ctidx) {
+		if (efi_image_region_add_checked(regs, efi, len, subsys_off,
+						 header_size - subsys_off, 0))
+			goto err;
+	} else {
+		/* Skip Certificates Table */
+		if (efi_image_region_add_checked(regs, efi, len, subsys_off,
+						 dir_off - subsys_off, 0))
+			goto err;
+		if (efi_image_region_add_checked(regs, efi, len, dir_end,
+						 header_size - dir_end, 0))
+			goto err;
 	}
 
+	bytes_hashed = header_size;
+
 	/* 2. Sections */
-	num_sections = nt->FileHeader.NumberOfSections;
-	sections = (void *)((uint8_t *)&nt->OptionalHeader +
-			    nt->FileHeader.SizeOfOptionalHeader);
+	sections = (void *)((uint8_t *)efi + sections_off);
 	sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections);
 	if (!sorted) {
 		log_err("%s: Out of memory\n", __func__);
@@ -516,14 +600,29 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 	qsort(sorted, num_sections, sizeof(sorted[0]), cmp_pe_section);
 
 	for (i = 0; i < num_sections; i++) {
+		size_t size;
+
 		if (!sorted[i]->SizeOfRawData)
 			continue;
 
-		size = (sorted[i]->SizeOfRawData + align - 1) & ~(align - 1);
-		efi_image_region_add(regs, efi + sorted[i]->PointerToRawData,
-				     efi + sorted[i]->PointerToRawData + size,
-				     0);
-		log_debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
+		if (!pe_range_ok(len, sorted[i]->PointerToRawData,
+				 sorted[i]->SizeOfRawData)) {
+			log_err("%s: Invalid section %d\n", __func__, i);
+			goto err;
+		}
+		/*
+		 * The padding to the file alignment ends with the image. Round
+		 * up in 64 bits, where the sum of two u32 cannot wrap around.
+		 */
+		size = min_t(u64, ALIGN((u64)sorted[i]->SizeOfRawData, align),
+			     len - sorted[i]->PointerToRawData);
+		if (efi_image_region_add_checked(regs, efi, len,
+						 sorted[i]->PointerToRawData,
+						 size, 0)) {
+			log_err("%s: Invalid section %d\n", __func__, i);
+			goto err;
+		}
+		log_debug("section[%d](%s): raw: 0x%x-0x%zx, virt: %x-%x\n",
 			  i, sorted[i]->Name,
 			  sorted[i]->PointerToRawData,
 			  sorted[i]->PointerToRawData + size,
@@ -534,25 +633,23 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 		bytes_hashed += size;
 	}
 	free(sorted);
+	sorted = NULL;
 
 	/* 3. Extra data excluding Certificates Table */
-	if (bytes_hashed + authsz < len) {
+	if (bytes_hashed < len && authsz < len - bytes_hashed) {
 		log_debug("extra data for hash: %zu\n",
 			  len - (bytes_hashed + authsz));
-		efi_image_region_add(regs, efi + bytes_hashed,
-				     efi + len - authsz, 0);
+		if (efi_image_region_add_checked(regs, efi, len, bytes_hashed,
+						 len - bytes_hashed - authsz,
+						 0))
+			goto err;
 	}
 
 	/* Return Certificates Table */
 	if (authsz) {
-		if (len < authoff + authsz) {
-			log_err("%s: Size for auth too large: %u >= %zu\n",
-				__func__, authsz, len - authoff);
-			goto err;
-		}
-		if (authsz < sizeof(*auth)) {
+		if (authsz < sizeof(**auth)) {
 			log_err("%s: Size for auth too small: %u < %zu\n",
-				__func__, authsz, sizeof(*auth));
+				__func__, authsz, sizeof(**auth));
 			goto err;
 		}
 		*auth = efi + authoff;
@@ -569,6 +666,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
 	return true;
 
 err:
+	free(sorted);
 	free(regs);
 
 	return false;

[PATCH v3 03/11] efi_loader: reject a PE section whose end wraps around

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_load_pe() computes the end of each section as the sum of its
VirtualAddress and its size, both u32, so the sum wraps around before it
is widened to unsigned long. A section near 4 GiB then yields a small
virt_size, and the memset() and memcpy() of the section write far past
the allocated image.

Relative virtual addresses are 32 bit, so reject a section whose end
does not fit into 32 bits. barebox, whose EFI loader is based on this
file, fixed the same in b6598389d46d ("efi: loader: fix integer overflow
in PE virt_size calculation").

This runs before the image is rejected when Secure Boot fails to verify
it.

Fixes: cb149c66342c ("efi_loader: Add PE image loader")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_image_loader.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 82083cb73d9..b21947e979a 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -1048,9 +1048,21 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 	/* Calculate upper virtual address boundary */
 	for (i = num_sections - 1; i >= 0; i--) {
 		IMAGE_SECTION_HEADER *sec = &sections[i];
+		u32 end;
 
-		virt_size = max_t(unsigned long, virt_size,
-				  sec->VirtualAddress + section_size(sec));
+		/*
+		 * VirtualAddress and section_size() are both u32, so their sum
+		 * wraps around at 4 GiB, on 64bit systems too. A wrapped end
+		 * would make virt_size too small for the memset() and memcpy()
+		 * below.
+		 */
+		if (__builtin_add_overflow(sec->VirtualAddress,
+					   section_size(sec), &end)) {
+			log_err("Section %d virtual address overflow\n", i);
+			return EFI_LOAD_ERROR;
+		}
+
+		virt_size = max_t(unsigned long, virt_size, end);
 	}
 
 	/* Read 32/64bit specific header bits */

[PATCH v3 04/11] efi_loader: check PE section raw data against the image size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_load_pe() copies each section from PointerToRawData in the image
without checking that the copied bytes lie within the image, so a
section pointing near or past the end of the image is read out of
bounds.

Reject a section whose raw data does not lie within the image before the
image is allocated, also when VirtualSize is smaller and only part of
the raw data is copied. barebox, whose EFI loader is based on this file,
checks the copied part since 552a54e4d357 ("efi: loader: validate
section raw data bounds against image size").

This runs before the image is rejected when Secure Boot fails to verify
it.

Fixes: cb149c66342c ("efi_loader: Add PE image loader")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_image_loader.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index b21947e979a..c6e7d5fc599 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -1062,6 +1062,13 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 			return EFI_LOAD_ERROR;
 		}
 
+		/* The raw data of the section must lie within the image */
+		if (!pe_range_ok(efi_size, sec->PointerToRawData,
+				 sec->SizeOfRawData)) {
+			log_err("Section %d exceeds image size\n", i);
+			return EFI_LOAD_ERROR;
+		}
+
 		virt_size = max_t(unsigned long, virt_size, end);
 	}
 

[PATCH v3 05/11] efi_loader: make room for the PE headers in the loaded image

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_load_pe() copies the PE headers to the start of the allocated image.
The size it copies is not the size of the headers: it adds the size of a
whole 32bit NT header, which already holds an optional header, to
SizeOfOptionalHeader, and assumes that the NT header follows the DOS
header directly. Nor is it checked against the size of the allocation,
which only covers the sections. An image with many empty section
headers but little section data makes the copy write past the
allocation, and the copy reads past the end of a small image.

Copy the headers up to the end of the section table, which is already
checked to lie within the image, and allocate at least that much.

This runs before the image is rejected when Secure Boot fails to verify
it.

Fixes: 8458bf648fd7 ("efi_loader: set image_base and image_size to correct values")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_image_loader.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index c6e7d5fc599..a9e7fa0a74f 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -996,7 +996,6 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 			 struct efi_loaded_image *loaded_image_info)
 {
 	IMAGE_NT_HEADERS32 *nt;
-	IMAGE_DOS_HEADER *dos;
 	IMAGE_SECTION_HEADER *sections;
 	int num_sections;
 	void *efi_reloc;
@@ -1006,6 +1005,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 	int rel_idx = IMAGE_DIRECTORY_ENTRY_BASERELOC;
 	uint64_t image_base;
 	unsigned long virt_size = 0;
+	size_t hdr_size;
 	int supported = 0;
 	efi_status_t ret;
 
@@ -1072,6 +1072,13 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 		virt_size = max_t(unsigned long, virt_size, end);
 	}
 
+	/*
+	 * The PE headers, up to the end of the section table, are copied to
+	 * the start of the loaded image: make room for them.
+	 */
+	hdr_size = (void *)(sections + num_sections) - efi;
+	virt_size = max_t(unsigned long, virt_size, hdr_size);
+
 	/* Read 32/64bit specific header bits */
 	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
 		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
@@ -1128,11 +1135,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 #endif
 
 	/* Copy PE headers */
-	memcpy(efi_reloc, efi,
-	       sizeof(*dos)
-		 + sizeof(*nt)
-		 + nt->FileHeader.SizeOfOptionalHeader
-		 + num_sections * sizeof(IMAGE_SECTION_HEADER));
+	memcpy(efi_reloc, efi, hdr_size);
 
 	/* Load sections into RAM */
 	for (i = num_sections - 1; i >= 0; i--) {

[PATCH v3 06/11] efi_loader: check the relocation table against the image size

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_loader_relocate() checks each relocation block and entry against the
end of the relocation table and each target against the image size, but
the table itself is taken from the base relocation data directory as it
is: its address and size are not checked against the image, so the
blocks are read from wherever they point, up to 4 GiB past the image.
The directory is also read when NumberOfRvaAndSizes says the image has
none, in which case its bytes belong to the section table.

Use the base relocation directory only if the image has one, and reject
a relocation table that does not lie within the image, as EDK2's
PeCoffLoaderRelocateImage() does.

This runs before the image is rejected when Secure Boot fails to verify
it.

Fixes: cb149c66342c ("efi_loader: Add PE image loader")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 lib/efi_loader/efi_image_loader.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index a9e7fa0a74f..417282698fa 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -117,9 +117,17 @@ static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel,
 					unsigned long virt_size)
 {
 	unsigned long delta = (unsigned long)efi_reloc - pref_address;
+	unsigned long rel_off = (const char *)rel - (const char *)efi_reloc;
 	const IMAGE_BASE_RELOCATION *end;
 	int i;
 
+	/* The relocation table must lie within the image */
+	if (rel_size &&
+	    (rel_off > virt_size || rel_size > virt_size - rel_off)) {
+		log_debug("Relocation table exceeds image size\n");
+		return EFI_LOAD_ERROR;
+	}
+
 	if (delta == 0)
 		return EFI_SUCCESS;
 
@@ -1095,7 +1103,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 			goto err;
 		}
 		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
-		rel_size = opt->DataDirectory[rel_idx].Size;
+		rel_size = opt->NumberOfRvaAndSizes > rel_idx ?
+			   opt->DataDirectory[rel_idx].Size : 0;
 		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
 	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
@@ -1111,7 +1120,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 			goto err;
 		}
 		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
-		rel_size = opt->DataDirectory[rel_idx].Size;
+		rel_size = opt->NumberOfRvaAndSizes > rel_idx ?
+			   opt->DataDirectory[rel_idx].Size : 0;
 		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
 	} else {
 		log_err("Invalid optional header magic %x\n",

[PATCH v3 07/11] efi_loader: check certificate lengths in efi_image_authenticate()

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_image_authenticate() walks the certificate table of an image by
dwLength, rounded up to a multiple of 8, without checking it against
the table:

- a certificate no longer than its header is skipped, but a dwLength
  of 0, or one that wraps around to 0 when rounded up, does not
  advance the walk, so U-Boot hangs on such an image;
- a dwLength larger than the rest of the table makes
  pkcs7_parse_message() read past the end of the table and of the
  image.

Turn the check that skipped a certificate no longer than its header
into one that rejects the image when a certificate is no longer than
its header or longer than the rest of the table. EDK2's
DxeImageVerificationLib also rejects an image whose certificate table
holds a certificate shorter than its header or longer than the rest of
the table.

Fixes: 4540dabdcaca ("efi_loader: image_loader: support image authentication")
Fixes: 1a44b7059c18 ("efi_loader: image_loader: add a check against certificate type of authenticode")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v3:
- fold the new check and the old "dwLength too small" one into a
  single check (Ilias)

 lib/efi_loader/efi_image_loader.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 417282698fa..8f48d8fb5a5 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -808,14 +808,26 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
 	for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len;
 	     (u8 *)wincert < wincerts_end;
 	     wincert = (WIN_CERTIFICATE *)
-			((u8 *)wincert + ALIGN(wincert->dwLength, 8))) {
-		if ((u8 *)wincert + sizeof(*wincert) >= wincerts_end)
+			((u8 *)wincert + ALIGN((size_t)wincert->dwLength, 8))) {
+		size_t remaining = wincerts_end - (u8 *)wincert;
+
+		if (sizeof(*wincert) >= remaining)
 			break;
 
-		if (wincert->dwLength <= sizeof(*wincert)) {
-			log_debug("dwLength too small: %u < %zu\n",
-				  wincert->dwLength, sizeof(*wincert));
-			continue;
+		/*
+		 * dwLength includes the WIN_CERTIFICATE header. A certificate
+		 * must hold a signature after its header and end within the
+		 * certificate table. Reject an image with a corrupted
+		 * certificate table: a dwLength of 0 would not advance the
+		 * walk, and one past the end of the table would make the
+		 * signature run past it.
+		 */
+		if (wincert->dwLength <= sizeof(*wincert) ||
+		    wincert->dwLength > remaining) {
+			log_err("Invalid certificate length: %u\n",
+				wincert->dwLength);
+			ret = false;
+			goto out;
 		}
 
 		log_debug("WIN_CERTIFICATE_TYPE: 0x%x\n",

[PATCH v3 08/11] efi_loader: refuse a PE image of an unknown subsystem

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

efi_load_pe() loads an image whose Subsystem field is none of the UEFI
image types as an application: efi_set_code_and_data_type() only logs
"invalid image type". Such an image is then not measured by
tcg2_measure_pe_image(), and efi_start_image() does not measure its
invocation either, so with measured boot it runs without a trace in
PCR 4.

The UEFI specification defines subsystems 10, 11 and 12 for UEFI images,
and LoadImage() returns EFI_UNSUPPORTED for an image type that is not
supported. EDK2 does so in CoreLoadPeImage(). Do the same here, also
for IMAGE_SUBSYSTEM_EFI_ROM, which was given a runtime driver's memory
types but could not be measured either.

Fixes: 36b41a3cedd6 ("efi_loader: allocate correct memory type for EFI image")
Suggested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_image_loader.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 8f48d8fb5a5..67b83af0e8e 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -252,8 +252,10 @@ static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel,
  * @loaded_image_info:	image descriptor
  * @image_type:		field Subsystem of the optional header for
  *			Windows specific field
+ * Return:		status code, EFI_UNSUPPORTED if @image_type is not
+ *			one of the UEFI image types
  */
-static void efi_set_code_and_data_type(
+static efi_status_t efi_set_code_and_data_type(
 			struct efi_loaded_image *loaded_image_info,
 			uint16_t image_type)
 {
@@ -267,17 +269,15 @@ static void efi_set_code_and_data_type(
 		loaded_image_info->image_data_type = EFI_BOOT_SERVICES_DATA;
 		break;
 	case IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER:
-	case IMAGE_SUBSYSTEM_EFI_ROM:
 		loaded_image_info->image_code_type = EFI_RUNTIME_SERVICES_CODE;
 		loaded_image_info->image_data_type = EFI_RUNTIME_SERVICES_DATA;
 		break;
 	default:
 		log_err("invalid image type: %u\n", image_type);
-		/* Let's assume it is an application */
-		loaded_image_info->image_code_type = EFI_LOADER_CODE;
-		loaded_image_info->image_data_type = EFI_LOADER_DATA;
-		break;
+		return EFI_UNSUPPORTED;
 	}
+
+	return EFI_SUCCESS;
 }
 
 /**
@@ -1104,7 +1104,10 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
 		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
 		image_base = opt->ImageBase;
-		efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
+		ret = efi_set_code_and_data_type(loaded_image_info,
+						 opt->Subsystem);
+		if (ret != EFI_SUCCESS)
+			goto err;
 		handle->image_type = opt->Subsystem;
 		efi_reloc = efi_alloc_aligned_pages(virt_size,
 						    loaded_image_info->image_code_type,
@@ -1121,7 +1124,10 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
 		image_base = opt->ImageBase;
-		efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
+		ret = efi_set_code_and_data_type(loaded_image_info,
+						 opt->Subsystem);
+		if (ret != EFI_SUCCESS)
+			goto err;
 		handle->image_type = opt->Subsystem;
 		efi_reloc = efi_alloc_aligned_pages(virt_size,
 						    loaded_image_info->image_code_type,

[PATCH v3 09/11] efi_loader: do not start an image that cannot be measured

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

With TCG2 measured boot, efi_load_pe() measures each image into PCR 4
or 2. When the measurement finds no TPM, the image is refused. Any other
error is ignored, so an image whose hash cannot be computed, because
efi_image_parse() rejects it, or whose PCR could not be extended is
loaded and can be started without being measured. With the range
checks added to efi_image_parse() earlier in this series, more
malformed images take this path. Such an image runs with boot services
and can start the expected boot loader itself, which is measured as
usual, so the PCRs end up as for a good boot.

Mark an image whose measurement failed as not authenticated, as
efi_load_pe() does for an image that fails Secure Boot authentication,
so that LoadImage() returns EFI_SECURITY_VIOLATION and StartImage()
refuses to start it. A full event log is no error, as in EDK2's
DxeTpm2MeasureBootLib: the PCR has been extended all the same.

Fixes: 163a0d7e2cbd ("efi_loader: add PE/COFF image measurement")
Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v3:
- apply to main without Heinrich's "efi_loader: handling of missing
  TPM in efi_load_pe()": the new check comes before the one for a
  missing TPM, so that this patch applies with or without Heinrich's,
  and his applies on top of it (Ilias)

 lib/efi_loader/efi_image_loader.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 67b83af0e8e..b14065fed6c 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -1151,6 +1151,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
 #if IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)
 	/* Measure an PE/COFF image */
 	ret = tcg2_measure_pe_image(efi, efi_size, handle, loaded_image_info);
+	if (ret != EFI_SUCCESS && ret != EFI_SECURITY_VIOLATION &&
+	    ret != EFI_VOLUME_FULL) {
+		/*
+		 * The image cannot be hashed or the PCR cannot be extended.
+		 * Do not start an image that has not been measured. A full
+		 * event log is no error: the PCR has been extended all the same.
+		 */
+		log_err("PE image measurement failed\n");
+		handle->auth_status = EFI_IMAGE_AUTH_FAILED;
+	}
 	if (ret == EFI_SECURITY_VIOLATION) {
 		/*
 		 * TCG2 Protocol is installed but no TPM device found,

[PATCH v3 10/11] test: lib: add tests for the PE image checks

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

Add unit tests that pass malformed PE images to efi_check_pe(),
LoadImage() and efi_image_parse() and check that they are rejected:

- an image ending after a 32bit NT header;
- a section whose end wraps around 4 GiB;
- a section whose raw data runs past the end of the image, also when
  its VirtualSize is smaller;
- a relocation table past the end of the image;
- an image whose subsystem is not a UEFI image type;
- an image that cannot be measured, when TCG2 is installed, which may
  be loaded but must not be started;
- headers, an optional header, a section or a certificate table that do
  not fit, an optional header that holds fewer data directories than it
  declares or more than 16, a file alignment that is not a power of two,
  and sections sharing their raw data.

Check that PE32 and PE32+ images with 16, 6 or 5 data directories, an
empty section, a last section that is not padded, a file alignment of 0
and trailing data of an odd size still load and parse, and that the
loaded image holds all of the headers when they are longer than the
sections.

Each image is built at the start of a larger zeroed buffer, so that
without the checks most tests read the bytes after it and get a wrong
status instead of faulting. Without the checks the tests of the wrapped
section and of the relocation table far past the image crash, and the
test of the headers writes past the loaded image.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 test/lib/Makefile           |   2 +-
 test/lib/efi_image_loader.c | 501 ++++++++++++++++++++++++++++++++++++
 2 files changed, 502 insertions(+), 1 deletion(-)

diff --git a/test/lib/Makefile b/test/lib/Makefile
index 9fc4a1a9b5d..a67e53ca9c5 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -9,7 +9,7 @@ ifeq ($(CONFIG_XPL_BUILD),)
 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_LOADER) += efi_device_path.o efi_image_loader.o efi_memory.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_image_loader.c b/test/lib/efi_image_loader.c
new file mode 100644
index 00000000000..8b4fff4a6f9
--- /dev/null
+++ b/test/lib/efi_image_loader.c
@@ -0,0 +1,501 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test the checks of the PE image loader
+ *
+ * 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>
+
+#if defined(__aarch64__)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_ARM64
+#elif defined(__arm__)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_THUMB
+#elif defined(__x86_64__)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_AMD64
+#elif defined(__i386__)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_I386
+#elif defined(__riscv) && (__riscv_xlen == 64)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_RISCV64
+#elif defined(__riscv) && (__riscv_xlen == 32)
+#define TEST_MACHINE	IMAGE_FILE_MACHINE_RISCV32
+#else
+/* The loader supports no other machine: skip the tests that load images */
+#define TEST_MACHINE	0
+#endif
+
+#define TEST_FILE_ALIGN		0x200
+#define TEST_SECTION_ALIGN	0x1000
+/* Size of a valid image: headers and one section, each TEST_FILE_ALIGN */
+#define TEST_IMAGE_SIZE		(2 * TEST_FILE_ALIGN)
+
+/**
+ * struct test_image - a PE image at the start of buf
+ *
+ * @dos:	DOS header
+ * @nt:		NT headers, of a PE32 or a PE32+ image
+ * @opt32:	optional header of a PE32 image, else NULL
+ * @opt64:	optional header of a PE32+ image, else NULL
+ * @dirs:	data directories
+ * @sections:	section table
+ */
+struct test_image {
+	IMAGE_DOS_HEADER *dos;
+	IMAGE_NT_HEADERS32 *nt;
+	IMAGE_OPTIONAL_HEADER32 *opt32;
+	IMAGE_OPTIONAL_HEADER64 *opt64;
+	IMAGE_DATA_DIRECTORY *dirs;
+	IMAGE_SECTION_HEADER *sections;
+};
+
+/* Set a field of the optional header of a PE32 or a PE32+ image */
+#define opt_set(img, field, val)			\
+	do {						\
+		if ((img)->opt64)			\
+			(img)->opt64->field = (val);	\
+		else					\
+			(img)->opt32->field = (val);	\
+	} while (0)
+
+/*
+ * Images are built at the start of this buffer, which is larger than any of
+ * them. The bytes after an image are zero, so that they can be read without
+ * a fault if a size check is missing.
+ */
+static u8 buf[0x4000] __aligned(8);
+
+/**
+ * test_image_init() - set up a valid image at the start of buf
+ *
+ * The image is TEST_IMAGE_SIZE bytes long: the headers, padded to the file
+ * alignment, and one section of TEST_FILE_ALIGN bytes. Further sections are
+ * empty.
+ *
+ * @img:		image to set up
+ * @pe32plus:		true for a PE32+ image, false for a PE32 image
+ * @num_dirs:		number of data directories
+ * @num_sections:	number of section headers
+ */
+static void test_image_init(struct test_image *img, bool pe32plus,
+			    u32 num_dirs, int num_sections)
+{
+	size_t opt_size;
+
+	memset(buf, 0, sizeof(buf));
+	memset(img, 0, sizeof(*img));
+
+	img->dos = (void *)buf;
+	img->dos->e_magic = IMAGE_DOS_SIGNATURE;
+	img->dos->e_lfanew = sizeof(*img->dos);
+
+	img->nt = (void *)buf + img->dos->e_lfanew;
+	img->nt->Signature = IMAGE_NT_SIGNATURE;
+	img->nt->FileHeader.Machine = TEST_MACHINE;
+	img->nt->FileHeader.NumberOfSections = num_sections;
+
+	if (pe32plus) {
+		img->opt64 = (void *)&img->nt->OptionalHeader;
+		img->opt64->Magic = IMAGE_NT_OPTIONAL_HDR64_MAGIC;
+		img->dirs = img->opt64->DataDirectory;
+		opt_size = offsetof(IMAGE_OPTIONAL_HEADER64, DataDirectory);
+	} else {
+		img->opt32 = &img->nt->OptionalHeader;
+		img->opt32->Magic = IMAGE_NT_OPTIONAL_HDR32_MAGIC;
+		img->dirs = img->opt32->DataDirectory;
+		opt_size = offsetof(IMAGE_OPTIONAL_HEADER32, DataDirectory);
+	}
+	opt_size += num_dirs * sizeof(IMAGE_DATA_DIRECTORY);
+	img->nt->FileHeader.SizeOfOptionalHeader = opt_size;
+	img->sections = (void *)&img->nt->OptionalHeader + opt_size;
+
+	opt_set(img, AddressOfEntryPoint, TEST_SECTION_ALIGN);
+	opt_set(img, SectionAlignment, TEST_SECTION_ALIGN);
+	opt_set(img, FileAlignment, TEST_FILE_ALIGN);
+	opt_set(img, SizeOfImage, 2 * TEST_SECTION_ALIGN);
+	opt_set(img, SizeOfHeaders, TEST_FILE_ALIGN);
+	opt_set(img, Subsystem, IMAGE_SUBSYSTEM_EFI_APPLICATION);
+	opt_set(img, NumberOfRvaAndSizes, num_dirs);
+
+	memcpy(img->sections[0].Name, ".text", 5);
+	img->sections[0].Misc.VirtualSize = TEST_FILE_ALIGN;
+	img->sections[0].VirtualAddress = TEST_SECTION_ALIGN;
+	img->sections[0].SizeOfRawData = TEST_FILE_ALIGN;
+	img->sections[0].PointerToRawData = TEST_FILE_ALIGN;
+}
+
+/**
+ * load_image() - pass an image at the start of buf to LoadImage()
+ *
+ * If the image is loaded, unload it again.
+ *
+ * @size:	size of the image
+ * Return:	status code of LoadImage()
+ */
+static efi_status_t load_image(size_t size)
+{
+	efi_handle_t handle = NULL;
+	efi_status_t ret;
+
+	ret = EFI_CALL(efi_load_image(false, efi_root, NULL, buf, size,
+				      &handle));
+	if (handle)
+		EFI_CALL(efi_unload_image(handle));
+
+	return ret;
+}
+
+/**
+ * parse_image() - pass an image at the start of buf to efi_image_parse()
+ *
+ * @size:	size of the image
+ * Return:	result of efi_image_parse()
+ */
+static bool parse_image(size_t size)
+{
+	struct efi_image_regions *regs = NULL;
+	WIN_CERTIFICATE *auth = NULL;
+	size_t auth_len = 0;
+	bool ret;
+
+	ret = efi_image_parse(buf, size, &regs, &auth, &auth_len);
+	free(regs);
+
+	return ret;
+}
+
+/* The checks must let valid images through */
+static int lib_test_efi_image_valid(struct unit_test_state *uts)
+{
+	static const u32 num_dirs[] = { IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 6 };
+	struct test_image img;
+	int i, j;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	for (i = 0; i < 2; i++) {
+		for (j = 0; j < ARRAY_SIZE(num_dirs); j++) {
+			test_image_init(&img, i, num_dirs[j], 1);
+			ut_asserteq(true, parse_image(TEST_IMAGE_SIZE));
+			ut_asserteq_64(EFI_SUCCESS, load_image(TEST_IMAGE_SIZE));
+		}
+	}
+
+	/* An empty section, and trailing data of an odd size */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 2);
+	memcpy(img.sections[1].Name, ".bss", 4);
+	img.sections[1].Misc.VirtualSize = TEST_FILE_ALIGN;
+	img.sections[1].VirtualAddress = 2 * TEST_SECTION_ALIGN;
+	opt_set(&img, SizeOfImage, 3 * TEST_SECTION_ALIGN);
+	ut_asserteq(true, parse_image(TEST_IMAGE_SIZE + 3));
+	ut_asserteq_64(EFI_SUCCESS, load_image(TEST_IMAGE_SIZE + 3));
+
+	/* An image with five data directories has no relocation table */
+	test_image_init(&img, true, 5, 1);
+	ut_asserteq(true, parse_image(TEST_IMAGE_SIZE));
+	ut_asserteq_64(EFI_SUCCESS, load_image(TEST_IMAGE_SIZE));
+
+	/* The last section is not padded to the file alignment */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	img.sections[0].SizeOfRawData = TEST_FILE_ALIGN / 2;
+	ut_asserteq(true, parse_image(TEST_IMAGE_SIZE - TEST_FILE_ALIGN / 2));
+	ut_asserteq_64(EFI_SUCCESS,
+		       load_image(TEST_IMAGE_SIZE - TEST_FILE_ALIGN / 2));
+
+	/* A file alignment of 0 stands for the default of 512 */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	opt_set(&img, FileAlignment, 0);
+	ut_asserteq(true, parse_image(TEST_IMAGE_SIZE));
+	ut_asserteq_64(EFI_SUCCESS, load_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_image_valid, 0);
+
+/* efi_check_pe() must check that the image holds a 64bit NT header */
+static int lib_test_efi_check_pe_nt_header(struct unit_test_state *uts)
+{
+	struct test_image img;
+	size_t size;
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	ut_asserteq_64(EFI_SUCCESS, efi_check_pe(buf, TEST_IMAGE_SIZE, NULL));
+
+	/* The image ends after a 32bit NT header */
+	size = img.dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32);
+	ut_asserteq_64(EFI_INVALID_PARAMETER, efi_check_pe(buf, size, NULL));
+
+	/* The NT header offset lies past the end (wraps on 32bit hosts) */
+	img.dos->e_lfanew = 0xfffffff0;
+	ut_asserteq_64(EFI_INVALID_PARAMETER,
+		       efi_check_pe(buf, TEST_IMAGE_SIZE, NULL));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_check_pe_nt_header, 0);
+
+/* efi_load_pe() must reject a section ending past 4 GiB */
+static int lib_test_efi_load_pe_section_wrap(struct unit_test_state *uts)
+{
+	struct test_image img;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	/* VirtualAddress + VirtualSize wraps around to TEST_SECTION_ALIGN */
+	img.sections[0].VirtualAddress = 0xfffff000;
+	img.sections[0].Misc.VirtualSize = 0x2000;
+	ut_asserteq_64(EFI_LOAD_ERROR, load_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_section_wrap, 0);
+
+/* efi_load_pe() must check the raw data of a section against the image */
+static int lib_test_efi_load_pe_section_raw(struct unit_test_state *uts)
+{
+	struct test_image img;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	/* The raw data of the section runs past the end of the image */
+	img.sections[0].SizeOfRawData = 2 * TEST_FILE_ALIGN;
+	img.sections[0].Misc.VirtualSize = 2 * TEST_FILE_ALIGN;
+	ut_asserteq_64(EFI_LOAD_ERROR, load_image(TEST_IMAGE_SIZE));
+
+	/* Only the part copied for the smaller VirtualSize lies within it */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	img.sections[0].SizeOfRawData = 2 * TEST_FILE_ALIGN;
+	ut_asserteq_64(EFI_LOAD_ERROR, load_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_section_raw, 0);
+
+/* efi_load_pe() must make room for the headers in the loaded image */
+static int lib_test_efi_load_pe_headers(struct unit_test_state *uts)
+{
+	struct efi_loaded_image *info;
+	struct efi_handler *handler;
+	efi_handle_t handle = NULL;
+	struct test_image img;
+	int num_sections, i;
+	size_t table_end, hdr_size, size;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	/*
+	 * Enough empty sections for the section table to end past the first
+	 * page, while the only section with data is TEST_FILE_ALIGN bytes
+	 * long at virtual address 0: the sections alone fit into one page.
+	 */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	num_sections = (EFI_PAGE_SIZE - ((void *)img.sections - (void *)buf)) /
+		       sizeof(IMAGE_SECTION_HEADER) + 2;
+	table_end = (void *)&img.sections[num_sections] - (void *)buf;
+	hdr_size = ALIGN(table_end, TEST_FILE_ALIGN);
+	size = hdr_size + TEST_FILE_ALIGN;
+	ut_assert(size <= sizeof(buf));
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES,
+			num_sections);
+	opt_set(&img, SizeOfHeaders, hdr_size);
+	opt_set(&img, AddressOfEntryPoint, 0);
+	img.sections[0].VirtualAddress = 0;
+	img.sections[0].PointerToRawData = hdr_size;
+	for (i = 1; i < num_sections; i++)
+		memcpy(img.sections[i].Name, ".bss", 4);
+
+	ut_asserteq_64(EFI_SUCCESS,
+		       EFI_CALL(efi_load_image(false, efi_root, NULL, buf, size,
+					       &handle)));
+	ut_asserteq_64(EFI_SUCCESS, efi_search_protocol(handle,
+							&efi_guid_loaded_image,
+							&handler));
+	info = handler->protocol_interface;
+	ut_assert(info->image_size >= table_end);
+	/* The section at virtual address 0 overwrites the first bytes */
+	ut_asserteq_mem(buf + TEST_FILE_ALIGN,
+			info->image_base + TEST_FILE_ALIGN,
+			table_end - TEST_FILE_ALIGN);
+	ut_asserteq_64(EFI_SUCCESS, EFI_CALL(efi_unload_image(handle)));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_headers, 0);
+
+/* efi_loader_relocate() must check the relocation table against the image */
+static int lib_test_efi_load_pe_reloc_table(struct unit_test_state *uts)
+{
+	struct test_image img;
+	IMAGE_DATA_DIRECTORY *dir;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	dir = &img.dirs[IMAGE_DIRECTORY_ENTRY_BASERELOC];
+	/* The relocation table starts in the section and ends past it */
+	dir->VirtualAddress = TEST_SECTION_ALIGN;
+	dir->Size = TEST_SECTION_ALIGN;
+	ut_asserteq_64(EFI_LOAD_ERROR, load_image(TEST_IMAGE_SIZE));
+
+	/* The relocation table lies far past the end of the image */
+	dir->VirtualAddress = 0xfffff000;
+	dir->Size = sizeof(IMAGE_BASE_RELOCATION);
+	ut_asserteq_64(EFI_LOAD_ERROR, load_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_reloc_table, 0);
+
+/* efi_load_pe() must refuse an image whose subsystem is no UEFI image type */
+static int lib_test_efi_load_pe_subsystem(struct unit_test_state *uts)
+{
+	static const u16 subsystems[] = { 0, 3, IMAGE_SUBSYSTEM_EFI_ROM };
+	struct test_image img;
+	int i, j;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+
+	for (i = 0; i < 2; i++) {
+		for (j = 0; j < ARRAY_SIZE(subsystems); j++) {
+			test_image_init(&img, i,
+					IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+			opt_set(&img, Subsystem, subsystems[j]);
+			ut_asserteq_64(EFI_UNSUPPORTED,
+				       load_image(TEST_IMAGE_SIZE));
+		}
+	}
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_subsystem, 0);
+
+/* efi_load_pe() must not let an image that cannot be measured start */
+static int lib_test_efi_load_pe_measure(struct unit_test_state *uts)
+{
+	const efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+	struct efi_handler *handler;
+	struct test_image img;
+
+	if (!TEST_MACHINE)
+		return -EAGAIN;
+
+	ut_asserteq_64(EFI_SUCCESS, efi_init_obj_list());
+	if (efi_search_protocol(efi_root, &tcg2_guid, &handler) != EFI_SUCCESS)
+		return -EAGAIN;
+
+	/*
+	 * LoadImage() takes a file alignment that is not a power of two, but
+	 * the image cannot be hashed for its measurement: it must be marked
+	 * as not authenticated, so that StartImage() refuses it.
+	 */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	opt_set(&img, FileAlignment, 3 * TEST_FILE_ALIGN / 2);
+	ut_asserteq_64(EFI_SECURITY_VIOLATION, load_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_load_pe_measure, 0);
+
+/* efi_image_parse() must check the PE headers against the image */
+static int lib_test_efi_image_parse_headers(struct unit_test_state *uts)
+{
+	struct test_image img;
+
+	/* The headers run past the end of the image */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	opt_set(&img, SizeOfHeaders, 2 * TEST_IMAGE_SIZE);
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	/* The optional header is shorter than its fixed fields */
+	test_image_init(&img, true, 0, 1);
+	img.nt->FileHeader.SizeOfOptionalHeader = 0x40;
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	/*
+	 * The optional header holds fewer data directories than it declares:
+	 * the certificate table entry would be read from the section table,
+	 * so clear the section name read as the entry.
+	 */
+	test_image_init(&img, false, IMAGE_DIRECTORY_ENTRY_SECURITY, 1);
+	opt_set(&img, NumberOfRvaAndSizes, IMAGE_NUMBEROF_DIRECTORY_ENTRIES);
+	memset(img.sections[0].Name, 0, sizeof(img.sections[0].Name));
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	/* The optional header holds more than 16 data directories */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES + 1, 1);
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_image_parse_headers, 0);
+
+/* efi_image_parse() must check each section against the image */
+static int lib_test_efi_image_parse_section(struct unit_test_state *uts)
+{
+	struct test_image img;
+
+	/* The raw data of the section runs past the end of the image */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	img.sections[0].SizeOfRawData = 2 * TEST_FILE_ALIGN;
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	/* The raw data of the section starts past the end of the image */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	img.sections[0].PointerToRawData = 0xfffffe00;
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	/* The file alignment is not a power of two */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	opt_set(&img, FileAlignment, 3 * TEST_FILE_ALIGN / 2);
+	ut_asserteq(false, parse_image(2 * TEST_IMAGE_SIZE));
+
+	/* Two sections share their raw data */
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 2);
+	img.sections[1] = img.sections[0];
+	img.sections[1].VirtualAddress = 2 * TEST_SECTION_ALIGN;
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_image_parse_section, 0);
+
+/* efi_image_parse() must check the certificate table against the image */
+static int lib_test_efi_image_parse_cert_table(struct unit_test_state *uts)
+{
+	struct test_image img;
+	IMAGE_DATA_DIRECTORY *dir;
+
+	test_image_init(&img, true, IMAGE_NUMBEROF_DIRECTORY_ENTRIES, 1);
+	dir = &img.dirs[IMAGE_DIRECTORY_ENTRY_SECURITY];
+	/* The end of the certificate table wraps around to 0x10 */
+	dir->VirtualAddress = 0xfffffff0;
+	dir->Size = 0x20;
+	ut_asserteq(false, parse_image(TEST_IMAGE_SIZE));
+
+	return 0;
+}
+LIB_TEST(lib_test_efi_image_parse_cert_table, 0);

[PATCH v3 11/11] test: py: test invalid certificate lengths in signed images

[Ana Bobillo]

From: Ana Bobillo <anabobillo3@gmail.com>

Add a Secure Boot test with two signed images whose first certificate has
an invalid length: zero, which made efi_image_authenticate() loop
forever, and one larger than the certificate table. Both images must be
rejected.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
 test/py/tests/test_efi_secboot/conftest.py    | 31 ++++++++++++++++
 test/py/tests/test_efi_secboot/test_signed.py | 36 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
index 0755497d46d..9858dd36f17 100644
--- a/test/py/tests/test_efi_secboot/conftest.py
+++ b/test/py/tests/test_efi_secboot/conftest.py
@@ -4,11 +4,36 @@
 
 """Fixture for UEFI secure boot test."""
 
+import struct
 from subprocess import call, check_call, CalledProcessError
 from tests import fs_helper
 import pytest
 from defs import *
 
+
+def set_cert_length(src, dst, length):
+    """Copy a signed image, setting the length of its first certificate.
+
+    Args:
+        src: Path of the signed image.
+        dst: Path of the image to write.
+        length: dwLength to set, or None for one past the certificate table.
+    """
+    with open(src, 'rb') as inf:
+        data = bytearray(inf.read())
+    pe_off = struct.unpack_from('<I', data, 0x3c)[0]
+    opt_off = pe_off + 24
+    magic = struct.unpack_from('<H', data, opt_off)[0]
+    # The data directories follow the PE32+ or the PE32 optional header
+    dirs_off = opt_off + (112 if magic == 0x20b else 96)
+    # The certificate table is data directory 4
+    cert_off, cert_size = struct.unpack_from('<II', data, dirs_off + 4 * 8)
+    if length is None:
+        length = cert_size + 8
+    struct.pack_into('<I', data, cert_off, length)
+    with open(dst, 'wb') as outf:
+        outf.write(data)
+
 @pytest.fixture(scope='function')
 def efi_boot_env(request, ubman):
     """Set up a file system to be used in UEFI secure boot test.
@@ -118,6 +143,12 @@ def efi_boot_env(request, ubman):
         # Create a corrupted signed image
         check_call('cd %s; sh %s/test/py/tests/test_efi_secboot/forge_image.sh helloworld.efi.signed helloworld_forged.efi.signed'
                    % (mnt_point, ubman.config.source_dir), shell=True)
+        # Create signed images with an invalid certificate length
+        set_cert_length('%s/helloworld.efi.signed' % mnt_point,
+                        '%s/helloworld_certlen0.efi.signed' % mnt_point, 0)
+        set_cert_length('%s/helloworld.efi.signed' % mnt_point,
+                        '%s/helloworld_certlen_long.efi.signed' % mnt_point,
+                        None)
         # Digest image
         check_call('cd %s; %shash-to-efi-sig-list helloworld.efi db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db db_hello.hash db_hello.auth'
                    % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py
index 9e6d3fdf8d8..d14f9b795d5 100644
--- a/test/py/tests/test_efi_secboot/test_signed.py
+++ b/test/py/tests/test_efi_secboot/test_signed.py
@@ -378,3 +378,39 @@ class TestEfiSignedImage(object):
                 'printenv -e -n PK'])
             assert 'Failed to set EFI variable' not in ''.join(output)
             assert '\"PK\" not defined' in ''.join(output)
+
+    def test_efi_signed_image_auth9(self, ubman, efi_boot_env):
+        """
+        Test Case 9 - Secure boot is in force,
+                      the length of the certificate in the signed image
+                      is invalid.
+                      Must be rejected.
+        """
+        ubman.restart_uboot()
+        disk_img = efi_boot_env
+        with ubman.log.section('Test Case 9a'):
+            # Test Case 9a, certificate length of zero
+            output = ubman.run_command_list([
+                'host bind 0 %s' % disk_img,
+                'fatload host 0:1 4000000 db.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000:$filesize db',
+                'fatload host 0:1 4000000 KEK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000:$filesize KEK',
+                'fatload host 0:1 4000000 PK.auth',
+                'setenv -e -nv -bs -rt -at -i 4000000:$filesize PK'])
+            assert 'Failed to set EFI variable' not in ''.join(output)
+            output = ubman.run_command_list([
+                'efidebug boot add -b 1 HELLO1 host 0:1 /helloworld_certlen0.efi.signed -s ""',
+                'efidebug boot order 1',
+                'efidebug test bootmgr'])
+            assert '\'HELLO1\' failed' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)
+
+        with ubman.log.section('Test Case 9b'):
+            # Test Case 9b, certificate longer than the certificate table
+            output = ubman.run_command_list([
+                'efidebug boot add -b 2 HELLO2 host 0:1 /helloworld_certlen_long.efi.signed -s ""',
+                'efidebug boot order 2',
+                'efidebug test bootmgr'])
+            assert '\'HELLO2\' failed' in ''.join(output)
+            assert 'efi_bootmgr_load() returned: 26' in ''.join(output)