diff --git a/src/boot/boot.c b/src/boot/boot.c index dbe7f0c8ab6..c2f7e9b5008 100644 --- a/src/boot/boot.c +++ b/src/boot/boot.c @@ -2046,8 +2046,8 @@ static bool is_sd_boot(EFI_FILE *root_dir, const char16_t *loader_path) { return false; _cleanup_free_ PeSectionHeader *section_table = NULL; - size_t n_section_table; - err = pe_section_table_from_file(handle, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_file(handle, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return false; @@ -2058,6 +2058,7 @@ static bool is_sd_boot(EFI_FILE *root_dir, const char16_t *loader_path) { section_names, /* profile= */ UINT_MAX, /* validate_base= */ 0, + size_in_memory, vector); if (vector[0].memory_size != STRLEN(SD_MAGIC)) return false; @@ -2320,8 +2321,8 @@ static void boot_entry_add_type2( /* Load section table once */ _cleanup_free_ PeSectionHeader *section_table = NULL; - size_t n_section_table; - err = pe_section_table_from_file(handle, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_file(handle, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return; @@ -2333,6 +2334,7 @@ static void boot_entry_add_type2( section_names, /* profile= */ UINT_MAX, /* validate_base= */ 0, + size_in_memory, base_sections); /* and now iterate through possible profiles, and create a menu item for each profile we find */ @@ -2348,6 +2350,7 @@ static void boot_entry_add_type2( section_names, profile, /* validate_base= */ 0, + size_in_memory, sections); if (err != EFI_SUCCESS && profile > 0) /* It's fine if there's no .profile for the first profile */ @@ -3100,7 +3103,7 @@ static EFI_STATUS call_image_start( if (err == EFI_UNSUPPORTED && entry->type == LOADER_LINUX) { uint32_t compat_address; - err = pe_kernel_info(loaded_image->ImageBase, /* ret_entry_point= */ NULL, &compat_address, + err = pe_kernel_info(loaded_image->ImageBase, loaded_image->ImageSize, /* ret_entry_point= */ NULL, &compat_address, /* ret_size_in_memory= */ NULL, /* ret_section_alignment= */ NULL); if (err != EFI_SUCCESS) { diff --git a/src/boot/device-path-util.c b/src/boot/device-path-util.c index 5243e81e5df..73c3933299c 100644 --- a/src/boot/device-path-util.c +++ b/src/boot/device-path-util.c @@ -25,6 +25,10 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE const EFI_DEVICE_PATH *end_node = device_path_find_end_node(dp); size_t file_size = strsize16(file); + /* The node Length is a uint16_t, so refuse a path that would not fit. */ + if (file_size > UINT16_MAX - sizeof(FILEPATH_DEVICE_PATH)) + return EFI_INVALID_PARAMETER; + size_t dp_size = (uint8_t *) end_node - (uint8_t *) dp; /* Make a copy that can also hold a file media device path. */ @@ -55,6 +59,9 @@ EFI_STATUS make_url_device_path(const char16_t *url, EFI_DEVICE_PATH **ret) { return EFI_INVALID_PARAMETER; size_t l = strlen8(u); + /* The node Length is a uint16_t, so refuse a URL that would not fit. */ + if (l > UINT16_MAX - offsetof(URI_DEVICE_PATH, Uri)) + return EFI_INVALID_PARAMETER; size_t t = offsetof(URI_DEVICE_PATH, Uri) + l + sizeof(EFI_DEVICE_PATH); EFI_DEVICE_PATH *dp = xmalloc(t); @@ -177,6 +184,23 @@ EFI_DEVICE_PATH *device_path_replace_node( return ret; } +bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size) { + if (!dp) + return false; + + /* Validate against the known size so a truncated/corrupt path can't make us read out of bounds. */ + for (;;) { + if (size < sizeof(EFI_DEVICE_PATH)) + return false; + if (dp->Length < sizeof(EFI_DEVICE_PATH) || dp->Length > size) + return false; + if (device_path_is_end(dp)) + return true; + size -= dp->Length; + dp = (const EFI_DEVICE_PATH *) ((const uint8_t *) dp + dp->Length); + } +} + size_t device_path_size(const EFI_DEVICE_PATH *dp) { const EFI_DEVICE_PATH *i = ASSERT_PTR(dp); diff --git a/src/boot/device-path-util.h b/src/boot/device-path-util.h index b02cf3146a9..8a993f27fe9 100644 --- a/src/boot/device-path-util.h +++ b/src/boot/device-path-util.h @@ -13,6 +13,9 @@ EFI_DEVICE_PATH *device_path_replace_node( static inline EFI_DEVICE_PATH *device_path_next_node(const EFI_DEVICE_PATH *dp) { assert(dp); + /* The node Length includes the 4-byte header, so a well-formed node is at least that long. Paths + * coming from untrusted sources must be checked with device_path_is_valid() before being walked. */ + assert(dp->Length >= sizeof(EFI_DEVICE_PATH)); return (EFI_DEVICE_PATH *) ((uint8_t *) dp + dp->Length); } @@ -30,4 +33,8 @@ static inline bool device_path_is_end(const EFI_DEVICE_PATH *dp) { size_t device_path_size(const EFI_DEVICE_PATH *dp); +/* Validates that a device path is well-formed and fully contained within the given size, terminated by an + * end node. Use on paths from untrusted sources (e.g. EFI variables) before walking them. */ +bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size); + EFI_DEVICE_PATH *device_path_dup(const EFI_DEVICE_PATH *dp); diff --git a/src/boot/efi-efivars.c b/src/boot/efi-efivars.c index c1ae4252c3c..6af325966f9 100644 --- a/src/boot/efi-efivars.c +++ b/src/boot/efi-efivars.c @@ -191,6 +191,16 @@ EFI_STATUS efivar_get_raw_full( size_t size = 0; err = RT->GetVariable((char16_t *) name, (EFI_GUID *) vendor, NULL, &size, NULL); + if (err == EFI_SUCCESS) { + /* The variable exists but is empty, initialize return parameters */ + if (ret_attributes) + *ret_attributes = 0; + if (ret_data) + *ret_data = NULL; + if (ret_size) + *ret_size = 0; + return EFI_SUCCESS; + } if (err != EFI_BUFFER_TOO_SMALL) return err; @@ -222,6 +232,9 @@ EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, b if (err != EFI_SUCCESS) return err; + if (size == 0) + return EFI_BUFFER_TOO_SMALL; + if (ret) *ret = *b > 0; diff --git a/src/boot/efi-string.c b/src/boot/efi-string.c index 30644638627..efe1553696d 100644 --- a/src/boot/efi-string.c +++ b/src/boot/efi-string.c @@ -500,7 +500,7 @@ char* line_get_key_value(char *s, const char *sep, size_t *pos, char **ret_key, value++; /* unquote */ - if (strchr8(QUOTES, value[0]) && line[linelen - 1] == value[0]) { + if (value[0] != '\0' && strchr8(QUOTES, value[0]) && line[linelen - 1] == value[0]) { value++; line[linelen - 1] = '\0'; } diff --git a/src/boot/linux.c b/src/boot/linux.c index bd71ada4835..67ed8599931 100644 --- a/src/boot/linux.c +++ b/src/boot/linux.c @@ -153,6 +153,25 @@ static EFI_STATUS memory_mark_rw_nx(EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto, return EFI_SUCCESS; } +typedef struct CleanupNxSections { + EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto; + struct iovec *sections; + size_t n_sections; +} CleanupNxSections; + +static void cleanup_nx_sections(CleanupNxSections *c) { + assert(c); + + /* Restore the code sections that were marked RO+X back to RW+NX before their backing pages are + * freed: EDK2 requires freed buffers to be writable and non-executable (it may overwrite them with + * a fixed pattern), otherwise FreePages() crashes. */ + if (c->memory_proto) + for (size_t i = 0; i < c->n_sections; i++) + (void) memory_mark_rw_nx(c->memory_proto, &c->sections[i]); + + free(c->sections); +} + EFI_STATUS linux_exec( EFI_HANDLE parent_image, const char16_t *cmdline, @@ -167,7 +186,7 @@ EFI_STATUS linux_exec( assert(iovec_is_set(kernel)); assert(iovec_is_valid(initrd)); - err = pe_kernel_info(kernel->iov_base, &entry_point, &compat_entry_point, &kernel_size_in_memory, §ion_alignment); + err = pe_kernel_info(kernel->iov_base, kernel->iov_len, &entry_point, &compat_entry_point, &kernel_size_in_memory, §ion_alignment); #if defined(__i386__) || defined(__x86_64__) if (err == EFI_UNSUPPORTED) /* Kernel is too old to support LINUX_INITRD_MEDIA_GUID, try the deprecated EFI handover @@ -242,8 +261,6 @@ EFI_STATUS linux_exec( * https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/ * https://www.kraxel.org/blog/2023/12/uefi-nx-linux-boot/ */ EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto = NULL; - _cleanup_free_ struct iovec *nx_sections = NULL; - size_t n_nx_sections = 0; if (pe_kernel_check_nx_compat(kernel->iov_base)) { /* LocateProtocol() is not quite that quick if you have many protocols, so only look for it @@ -259,8 +276,7 @@ EFI_STATUS linux_exec( const PeSectionHeader *headers; size_t n_headers; - /* Do we need to validate anything here? the len? */ - err = pe_section_table_from_base(kernel->iov_base, &headers, &n_headers); + err = pe_section_table_from_base(kernel->iov_base, kernel->iov_len, &headers, &n_headers, /* ret_size_in_memory= */ NULL); if (err != EFI_SUCCESS) return log_error_status(err, "Cannot read sections: %m"); @@ -277,6 +293,12 @@ EFI_STATUS linux_exec( /* addr= */ 0); uint8_t* loaded_kernel = PHYSICAL_ADDRESS_TO_POINTER(loaded_kernel_pages.addr); + + /* Any code section marked RO+X must be reverted to RW+NX before the backing pages are freed. */ + _cleanup_(cleanup_nx_sections) CleanupNxSections nx_restore = { + .memory_proto = memory_proto, + }; + FOREACH_ARRAY(h, headers, n_headers) { if (h->PointerToRelocations != 0) return log_error_status(EFI_LOAD_ERROR, "Inner kernel image contains sections with relocations, which we do not support."); @@ -289,6 +311,10 @@ EFI_STATUS linux_exec( return log_error_status(EFI_LOAD_ERROR, "Section would write outside of memory"); if (h->SizeOfRawData > h->VirtualSize) return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, raw data size is greater than virtual size"); + if (UINT32_MAX - h->VirtualAddress < h->VirtualSize) + return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, VirtualSize + VirtualAddress overflows"); + if (h->VirtualAddress + h->VirtualSize > kernel_size_in_memory) + return log_error_status(EFI_LOAD_ERROR, "Section virtual size would write outside of memory"); if (UINT32_MAX - h->PointerToRawData < h->SizeOfRawData) return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, PointerToRawData + SizeOfRawData overflows"); if (h->PointerToRawData + h->SizeOfRawData > kernel->iov_len) @@ -301,15 +327,14 @@ EFI_STATUS linux_exec( /* Not a code section? Nothing to do, leave as-is. */ if (memory_proto && (h->Characteristics & (PE_CODE|PE_EXECUTE))) { - nx_sections = xrealloc(nx_sections, n_nx_sections * sizeof(struct iovec), (n_nx_sections + 1) * sizeof(struct iovec)); - nx_sections[n_nx_sections].iov_base = loaded_kernel + h->VirtualAddress; - nx_sections[n_nx_sections].iov_len = h->VirtualSize; + /* Record the section for cleanup before marking it RO+X: if memory_mark_ro_x() + * fails after partially applying the attributes, cleanup still reverts them. */ + nx_restore.sections = xrealloc(nx_restore.sections, nx_restore.n_sections * sizeof(struct iovec), (nx_restore.n_sections + 1) * sizeof(struct iovec)); + nx_restore.sections[nx_restore.n_sections++] = IOVEC_MAKE(loaded_kernel + h->VirtualAddress, h->VirtualSize); - err = memory_mark_ro_x(memory_proto, &nx_sections[n_nx_sections]); + err = memory_mark_ro_x(memory_proto, &nx_restore.sections[nx_restore.n_sections - 1]); if (err != EFI_SUCCESS) return err; - - ++n_nx_sections; } } @@ -330,8 +355,12 @@ EFI_STATUS linux_exec( _cleanup_(cleanup_initrd) EFI_HANDLE initrd_handle = NULL; err = initrd_register(initrd, &initrd_handle); - if (err != EFI_SUCCESS) + if (err != EFI_SUCCESS) { + /* Restore the patched fields before kernel_file_path and loaded_kernel_pages are freed, + * otherwise the stub's own EFI_LOADED_IMAGE_PROTOCOL is left pointing at freed memory. */ + *parent_loaded_image = original_parent_loaded_image; return log_error_status(err, "Error registering initrd: %m"); + } log_wait(); @@ -349,11 +378,5 @@ EFI_STATUS linux_exec( /* Restore */ *parent_loaded_image = original_parent_loaded_image; - /* On failure we'll free the buffers. EDK2 requires the memory buffers to be writable and - * non-executable, as in some configurations it will overwrite them with a fixed pattern, so if the - * attributes are not restored FreePages() will crash. */ - for (size_t i = 0; i < n_nx_sections; i++) - (void) memory_mark_rw_nx(memory_proto, &nx_sections[i]); - return log_error_status(err, "Error starting kernel image: %m"); } diff --git a/src/boot/part-discovery.c b/src/boot/part-discovery.c index ab553a6530b..10ba5241254 100644 --- a/src/boot/part-discovery.c +++ b/src/boot/part-discovery.c @@ -60,7 +60,8 @@ static bool verify_gpt(/* const */ GptHeader *h, EFI_LBA lba_expected) { if (h->MyLBA != lba_expected) return false; - if ((h->SizeOfPartitionEntry % sizeof(EFI_PARTITION_ENTRY)) != 0) + if (h->SizeOfPartitionEntry < sizeof(EFI_PARTITION_ENTRY) || + (h->SizeOfPartitionEntry % sizeof(EFI_PARTITION_ENTRY)) != 0) return false; if (h->NumberOfPartitionEntries <= 0 || h->NumberOfPartitionEntries > 1024) diff --git a/src/boot/pe.c b/src/boot/pe.c index 872cb9e6220..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) { @@ -173,6 +187,54 @@ static size_t section_table_offset(const DosFileHeader *dos, const PeFileHeader return dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader) + pe->FileHeader.SizeOfOptionalHeader; } +static EFI_STATUS pe_headers_from_base( + const void *base, + size_t base_len, + bool allow_compatibility, + const DosFileHeader **ret_dos, + const PeFileHeader **ret_pe) { + + assert(base); + assert(ret_dos); + assert(ret_pe); + + /* Validate the DOS and PE headers against the buffer length */ + + if (base_len < sizeof(DosFileHeader)) + return EFI_LOAD_ERROR; + + const DosFileHeader *dos = (const DosFileHeader*) base; + if (!verify_dos(dos)) + return EFI_LOAD_ERROR; + + if (dos->ExeHeader > base_len || base_len - dos->ExeHeader < sizeof(PeFileHeader)) + return EFI_LOAD_ERROR; + + const PeFileHeader *pe = (const PeFileHeader*) ((const uint8_t*) base + dos->ExeHeader); + if (!verify_pe(dos, pe, allow_compatibility)) + return EFI_LOAD_ERROR; + + *ret_dos = dos; + *ret_pe = pe; + return EFI_SUCCESS; +} + +static bool pe_section_table_in_bounds( + const DosFileHeader *dos, + const PeFileHeader *pe, + size_t base_len) { + + assert(dos); + assert(pe); + + /* verify_pe() already bounded SizeOfOptionalHeader so this offset cannot overflow, and + * NumberOfSections is a uint16_t so the byte count cannot either. */ + size_t offset = section_table_offset(dos, pe); + size_t bytes = (size_t) pe->FileHeader.NumberOfSections * sizeof(PeSectionHeader); + + return offset <= base_len && base_len - offset >= bytes; +} + static bool pe_section_name_equal(const char *a, const char *b) { if (a == b) @@ -258,6 +320,7 @@ static void pe_locate_sections_internal( size_t n_section_table, const char *const section_names[], size_t validate_base, + size_t size_in_memory, const void *device_table, const Device *device, PeSectionVector sections[]) { @@ -288,6 +351,11 @@ static void pe_locate_sections_internal( if ((size_t) j->VirtualSize > size_max) continue; + /* The section's in-memory range must lie within the image, otherwise consumers + * reading it via memory_offset/memory_size would read past the loaded image. */ + if ((size_t) j->VirtualAddress + (size_t) j->VirtualSize > size_in_memory) + continue; + /* 2nd overflow check: ignore sections that are impossibly large also taking the * loaded base into account. */ if (validate_base != 0) { @@ -360,6 +428,7 @@ static void pe_locate_sections( size_t n_section_table, const char *const section_names[], size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]) { if (!looking_for_dtbauto_or_efifw(section_names)) @@ -368,6 +437,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, /* device_table= */ NULL, /* device= */ NULL, sections); @@ -387,6 +457,7 @@ static void pe_locate_sections( n_section_table, hwid_section_names, validate_base, + size_in_memory, /* device_table= */ NULL, /* device= */ NULL, hwids_section); @@ -403,6 +474,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); @@ -418,6 +490,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); @@ -433,12 +506,13 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); } -static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const PeFileHeader *pe) { +static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, size_t base_len, const PeFileHeader *pe) { /* The kernel may provide alternative PE entry points for different PE architectures. This allows * booting a 64-bit kernel on 32-bit EFI that is otherwise running on a 64-bit CPU. The locations of any * such compat entry points are located in a special PE section. */ @@ -448,16 +522,29 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const static const char *const section_names[] = { ".compat", NULL }; PeSectionVector vector[1] = {}; + + /* Make sure the section table lies within the buffer before pe_locate_sections() iterates it. */ + if (!pe_section_table_in_bounds(dos, pe, base_len)) + return 0; + pe_locate_sections( (const PeSectionHeader *) ((const uint8_t *) dos + section_table_offset(dos, pe)), pe->FileHeader.NumberOfSections, section_names, PTR_TO_SIZE(dos), + pe->OptionalHeader.SizeOfImage, vector); if (!PE_SECTION_VECTOR_IS_SET(vector)) /* not found */ return 0; + /* pe_locate_sections() bounded the section against SizeOfImage, the in-memory size. Here we read the + * section data straight from the file buffer 'dos', which may be smaller than SizeOfImage, so also + * require the section's data range to lie within base_len before scanning it. */ + if (vector[0].memory_offset > base_len || + vector[0].memory_size > base_len - vector[0].memory_offset) + return 0; + typedef struct { uint8_t type; uint8_t size; @@ -487,19 +574,18 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const EFI_STATUS pe_kernel_info( const void *base, + size_t base_len, uint32_t *ret_entry_point, uint32_t *ret_compat_entry_point, size_t *ret_size_in_memory, uint32_t *ret_section_alignment) { assert(base); - const DosFileHeader *dos = (const DosFileHeader *) base; - if (!verify_dos(dos)) - return EFI_LOAD_ERROR; - - const PeFileHeader *pe = (const PeFileHeader *) ((const uint8_t *) base + dos->ExeHeader); - if (!verify_pe(dos, pe, /* allow_compatibility= */ true)) - return EFI_LOAD_ERROR; + const DosFileHeader *dos; + const PeFileHeader *pe; + EFI_STATUS err = pe_headers_from_base(base, base_len, /* allow_compatibility= */ true, &dos, &pe); + if (err != EFI_SUCCESS) + return err; /* When allocating we need to also consider the virtual/uninitialized data sections, so parse it out * of the SizeOfImage field in the PE header and return it */ @@ -520,6 +606,10 @@ EFI_STATUS pe_kernel_info( return EFI_UNSUPPORTED; if (pe->FileHeader.Machine == TARGET_MACHINE_TYPE) { + /* The entry point is later called as ImageBase + entry_point, and only SizeOfImage + * bytes are allocated for the image, so reject an entry point outside of it. */ + if (pe->OptionalHeader.AddressOfEntryPoint >= size_in_memory) + return EFI_LOAD_ERROR; if (ret_entry_point) *ret_entry_point = pe->OptionalHeader.AddressOfEntryPoint; if (ret_compat_entry_point) @@ -531,10 +621,13 @@ EFI_STATUS pe_kernel_info( return EFI_SUCCESS; } - uint32_t compat_entry_point = get_compatibility_entry_address(dos, pe); + uint32_t compat_entry_point = get_compatibility_entry_address(dos, base_len, pe); if (compat_entry_point == 0) /* Image type not supported and no compat entry found. */ return EFI_UNSUPPORTED; + if (compat_entry_point >= size_in_memory) + /* Same as above: the compat entry point is called as ImageBase + entry_point. */ + return EFI_LOAD_ERROR; if (ret_entry_point) *ret_entry_point = 0; @@ -548,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 @@ -599,20 +689,20 @@ bool pe_kernel_check_nx_compat(const void *base) { EFI_STATUS pe_section_table_from_base( const void *base, + size_t base_len, const PeSectionHeader **ret_section_table, - size_t *ret_n_section_table) { + size_t *ret_n_section_table, + size_t *ret_size_in_memory) { assert(base); assert(ret_section_table); assert(ret_n_section_table); - const DosFileHeader *dos = (const DosFileHeader*) base; - if (!verify_dos(dos)) - return EFI_LOAD_ERROR; - - const PeFileHeader *pe = (const PeFileHeader*) ((const uint8_t*) base + dos->ExeHeader); - if (!verify_pe(dos, pe, /* allow_compatibility= */ false)) - return EFI_LOAD_ERROR; + const DosFileHeader *dos; + const PeFileHeader *pe; + EFI_STATUS err = pe_headers_from_base(base, base_len, /* allow_compatibility= */ false, &dos, &pe); + if (err != EFI_SUCCESS) + return err; assert_cc(sizeof(pe->FileHeader.NumberOfSections) == sizeof(uint16_t)); /* multiplication below cannot overflow */ @@ -620,14 +710,22 @@ EFI_STATUS pe_section_table_from_base( if (n_section_table * sizeof(PeSectionHeader) > SECTION_TABLE_BYTES_MAX) return EFI_OUT_OF_RESOURCES; + /* Make sure the section table lies within the buffer, so consumers iterating it don't read off the + * end. */ + if (!pe_section_table_in_bounds(dos, pe, base_len)) + return EFI_LOAD_ERROR; + *ret_section_table = (const PeSectionHeader*) ((const uint8_t*) base + section_table_offset(dos, pe)); *ret_n_section_table = n_section_table; + if (ret_size_in_memory) + *ret_size_in_memory = pe->OptionalHeader.SizeOfImage; return EFI_SUCCESS; } EFI_STATUS pe_memory_locate_sections( const void *base, + size_t base_len, const char *const section_names[], PeSectionVector sections[]) { @@ -638,8 +736,8 @@ EFI_STATUS pe_memory_locate_sections( assert(sections); const PeSectionHeader *section_table; - size_t n_section_table; - err = pe_section_table_from_base(base, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_base(base, base_len, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return err; @@ -648,6 +746,7 @@ EFI_STATUS pe_memory_locate_sections( n_section_table, section_names, PTR_TO_SIZE(base), + size_in_memory, sections); return EFI_SUCCESS; @@ -656,7 +755,8 @@ EFI_STATUS pe_memory_locate_sections( EFI_STATUS pe_section_table_from_file( EFI_FILE *handle, PeSectionHeader **ret_section_table, - size_t *ret_n_section_table) { + size_t *ret_n_section_table, + size_t *ret_size_in_memory) { EFI_STATUS err; size_t len; @@ -710,6 +810,8 @@ EFI_STATUS pe_section_table_from_file( *ret_section_table = TAKE_PTR(section_table); *ret_n_section_table = n_section_table; + if (ret_size_in_memory) + *ret_size_in_memory = pe.OptionalHeader.SizeOfImage; return EFI_SUCCESS; } @@ -777,6 +879,7 @@ EFI_STATUS pe_locate_profile_sections( const char* const section_names[], unsigned profile, size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]) { assert(section_table || n_section_table == 0); @@ -798,6 +901,7 @@ EFI_STATUS pe_locate_profile_sections( n, section_names, validate_base, + size_in_memory, sections); return EFI_SUCCESS; diff --git a/src/boot/pe.h b/src/boot/pe.h index 5c8dc86fe93..bc9963799f5 100644 --- a/src/boot/pe.h +++ b/src/boot/pe.h @@ -36,13 +36,16 @@ static inline bool PE_SECTION_VECTOR_IS_SET(const PeSectionVector *v) { EFI_STATUS pe_section_table_from_base( const void *base, + size_t base_len, const PeSectionHeader **ret_section_table, - size_t *ret_n_section_table); + size_t *ret_n_section_table, + size_t *ret_size_in_memory); EFI_STATUS pe_section_table_from_file( EFI_FILE *handle, PeSectionHeader **ret_section_table, - size_t *ret_n_section_table); + size_t *ret_n_section_table, + size_t *ret_size_in_memory); EFI_STATUS pe_locate_profile_sections( const PeSectionHeader section_table[], @@ -50,15 +53,18 @@ EFI_STATUS pe_locate_profile_sections( const char* const section_names[], unsigned profile, size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]); EFI_STATUS pe_memory_locate_sections( const void *base, + size_t base_len, const char *const section_names[], PeSectionVector sections[]); EFI_STATUS pe_kernel_info( const void *base, + size_t base_len, uint32_t *ret_entry_point, uint32_t *ret_compat_entry_point, size_t *ret_size_in_memory, diff --git a/src/boot/stub.c b/src/boot/stub.c index e69faca00b0..94c4ac49e4b 100644 --- a/src/boot/stub.c +++ b/src/boot/stub.c @@ -596,7 +596,7 @@ static EFI_STATUS load_addons( if (err != EFI_SUCCESS) return log_error_status(err, "Failed to find protocol in %ls: %m", items[i]); - err = pe_memory_locate_sections(loaded_addon->ImageBase, unified_sections, sections); + err = pe_memory_locate_sections(loaded_addon->ImageBase, loaded_addon->ImageSize, unified_sections, sections); if (err != EFI_SUCCESS) { log_error_status(err, "Unable to locate embedded .cmdline/.dtb/.dtbauto/.efifw/.initrd/.ucode sections in %ls, ignoring: %m", @@ -1099,8 +1099,8 @@ static EFI_STATUS find_sections( assert(sections); const PeSectionHeader *section_table; - size_t n_section_table; - err = pe_section_table_from_base(loaded_image->ImageBase, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_base(loaded_image->ImageBase, loaded_image->ImageSize, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return log_error_status(err, "Unable to locate PE section table: %m"); @@ -1111,6 +1111,7 @@ static EFI_STATUS find_sections( unified_sections, /* profile= */ UINT_MAX, /* validate_base= */ PTR_TO_SIZE(loaded_image->ImageBase), + size_in_memory, sections); if (err != EFI_SUCCESS) return log_error_status(err, "Unable to locate embedded base PE sections: %m"); @@ -1123,6 +1124,7 @@ static EFI_STATUS find_sections( unified_sections, profile, /* validate_base= */ PTR_TO_SIZE(loaded_image->ImageBase), + size_in_memory, sections); if (err != EFI_SUCCESS && !(err == EFI_NOT_FOUND && profile == 0)) /* the first profile is implied if it doesn't exist */ return log_error_status(err, "Unable to locate embedded per-profile PE sections: %m"); diff --git a/src/boot/test-efi-string.c b/src/boot/test-efi-string.c index 7633534dd4c..5805cb30241 100644 --- a/src/boot/test-efi-string.c +++ b/src/boot/test-efi-string.c @@ -590,6 +590,7 @@ TEST(line_get_key_value) { " also\tused \r\n" "for \"the conf\"\n" "format\t !!"; + char s3[] = "ID="; size_t pos = 0; char *key, *value; @@ -611,6 +612,11 @@ TEST(line_get_key_value) { ASSERT_TRUE(streq8(value, " stripping # with comments")); ASSERT_NULL(line_get_key_value(s1, "=", &pos, &key, &value)); + pos = 0; + ASSERT_NOT_NULL(line_get_key_value(s3, "=", &pos, &key, &value)); + ASSERT_TRUE(streq8(key, "ID")); + ASSERT_TRUE(streq8(value, "")); + pos = 0; ASSERT_NOT_NULL(line_get_key_value(s2, " \t", &pos, &key, &value)); ASSERT_TRUE(streq8(key, "this")); diff --git a/src/boot/vmm.c b/src/boot/vmm.c index e571a3990de..09c121b9884 100644 --- a/src/boot/vmm.c +++ b/src/boot/vmm.c @@ -62,7 +62,7 @@ bool is_direct_boot(EFI_HANDLE device) { EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { _cleanup_free_ EFI_HANDLE *handles = NULL; size_t n_handles; - EFI_STATUS err, dp_err; + EFI_STATUS err; assert(ret_vmm_dev); assert(ret_vmm_dir); @@ -79,9 +79,15 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { for (size_t order = 0;; order++) { _cleanup_free_ EFI_DEVICE_PATH *dp = NULL; + size_t dp_size = 0; _cleanup_free_ char16_t *order_str = xasprintf("VMMBootOrder%04zx", order); - dp_err = efivar_get_raw(MAKE_GUID_PTR(VMM_BOOT_ORDER), order_str, (void**) &dp, NULL); + err = efivar_get_raw(MAKE_GUID_PTR(VMM_BOOT_ORDER), order_str, (void**) &dp, &dp_size); + + /* Drop the device path from the (untrusted) EFI variable if it doesn't validate, so the + * check below simply has to test whether it is set. */ + if (err == EFI_SUCCESS && !device_path_is_valid(dp, dp_size)) + dp = mfree(dp); for (size_t i = 0; i < n_handles; i++) { _cleanup_file_close_ EFI_FILE *root_dir = NULL, *efi_dir = NULL; @@ -92,8 +98,8 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { if (err != EFI_SUCCESS) return err; - /* check against VMMBootOrderNNNN (if set) */ - if (dp_err == EFI_SUCCESS && !device_path_startswith(fs, dp)) + /* check against VMMBootOrderNNNN (if set and valid) */ + if (dp && !device_path_startswith(fs, dp)) continue; err = open_volume(handles[i], &root_dir); @@ -112,7 +118,7 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { return EFI_SUCCESS; } - if (dp_err != EFI_SUCCESS) + if (!dp) return EFI_NOT_FOUND; } assert_not_reached();