From: Ana Bobillo <[email protected]>
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 <[email protected]>
Reviewed-by: Ilias Apalodimas <[email protected]>
---
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 7be5b6f2a9b..f81ccd62ab1 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 = §ions[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 */
