From 918e8f9dd91e5f51cd06b599ef3cbe534aaff28d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 23:12:54 +0100 Subject: [PATCH] boot: require a minimum PE optional header size in verify_pe() verify_pe() only checked SizeOfOptionalHeader against a SIZE_MAX wrap (a clause that, given SizeOfOptionalHeader is a uint16_t, can never reject anything) and never read NumberOfRvaAndSizes. But pe_kernel_info(), pe_kernel_check_nx_compat() and pe_kernel_check_no_relocation() then read SizeOfImage, AddressOfEntryPoint, DllCharacteristics and the base relocation data directory entry from the optional header. Require SizeOfOptionalHeader to be large enough to contain everything down to the base relocation data directory entry, and require the image to declare that many data directory entries. Follow-up for bacc2ed0d5bb10de5d37a1df73c061247f005b59 --- src/boot/pe.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/boot/pe.c b/src/boot/pe.c index 5e4c07e1610..b7974b33a50 100644 --- a/src/boot/pe.c +++ b/src/boot/pe.c @@ -140,6 +140,9 @@ typedef struct PeFileHeader { #define SECTION_TABLE_BYTES_MAX (16U * 1024U * 1024U) +/* https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-data-directories-image-only */ +#define BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY 5 + static bool verify_dos(const DosFileHeader *dos) { assert(dos); @@ -163,7 +166,18 @@ static bool verify_pe( (allow_compatibility && pe->FileHeader.Machine == TARGET_MACHINE_TYPE_COMPATIBILITY)) && pe->FileHeader.NumberOfSections > 0 && IN_SET(pe->OptionalHeader.Magic, OPTHDR32_MAGIC, OPTHDR64_MAGIC) && - pe->FileHeader.SizeOfOptionalHeader < SIZE_MAX - (dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader)); + pe->FileHeader.SizeOfOptionalHeader < SIZE_MAX - (dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader)) && + /* The optional header must be large enough to actually contain every field we read from + * it later (the deepest being the base relocation data directory entry), and must declare + * at least that many data directory entries. */ + pe->FileHeader.SizeOfOptionalHeader >= + (pe->OptionalHeader.Magic == OPTHDR32_MAGIC ? + offsetof(PeOptionalHeader, DataDirectory32) : + offsetof(PeOptionalHeader, DataDirectory64)) + + (BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY + 1) * sizeof(PeImageDataDirectory) && + (pe->OptionalHeader.Magic == OPTHDR32_MAGIC ? + pe->OptionalHeader.NumberOfRvaAndSizes32 : + pe->OptionalHeader.NumberOfRvaAndSizes64) > BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY; } static size_t section_table_offset(const DosFileHeader *dos, const PeFileHeader *pe) { @@ -627,9 +641,6 @@ EFI_STATUS pe_kernel_info( return EFI_SUCCESS; } -/* https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-data-directories-image-only */ -#define BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY 5 - /* We do not expect PE inner kernels to have any relocations. However that might be wrong for some * architectures, or it might change in the future. If the case of relocation arise, we should transform this * function in a function applying the relocations. However for now, since it would not be exercised and -- 2.47.3