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
