From 0a17be0be65e81c650b3e8e7a50460f9aabf8aca Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Wed, 2 Nov 2022 10:39:43 -0600 Subject: [PATCH 1/2] load_cert_file: Fix stack issue 0214cd9cef5a fixes a NULL pointer dereference problem, it introduces two new problems. First it incorrectly assumes li.FilePath is a string. Second, it puts EFI_LOADED_IMAGE li on the stack. It has been found that not all archectures can handle this being on the stack. The shim_li variable will be setup properly from the read_image call. Use the global shim_li variable instead when calling verify_image. Signed-off-by: Eric Snowberg --- shim.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/shim.c b/shim.c index 27b74ce06..0d919ceb8 100644 --- a/shim.c +++ b/shim.c @@ -1395,7 +1395,6 @@ EFI_STATUS load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) { EFI_STATUS efi_status; - EFI_LOADED_IMAGE li; PE_COFF_LOADER_IMAGE_CONTEXT context; EFI_IMAGE_SECTION_HEADER *Section; EFI_SIGNATURE_LIST *certlist; @@ -1410,10 +1409,7 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) if (EFI_ERROR(efi_status)) return efi_status; - memset(&li, 0, sizeof(li)); - memcpy(&li.FilePath[0], filename, MIN(StrSize(filename), sizeof(li.FilePath))); - - efi_status = verify_image(data, datasize, &li, &context); + efi_status = verify_image(data, datasize, shim_li, &context); if (EFI_ERROR(efi_status)) return efi_status; From ac082bae5c0843a3895730c2732f8791cd8a289d Mon Sep 17 00:00:00 2001 From: Eric Snowberg Date: Wed, 2 Nov 2022 10:45:23 -0600 Subject: [PATCH 2/2] load_cert_file: Use EFI RT memory function Use the EFI RT memory function CopyMem instead of memcpy in load_cert_file. Signed-off-by: Eric Snowberg --- shim.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shim.c b/shim.c index 0d919ceb8..4437898af 100644 --- a/shim.c +++ b/shim.c @@ -1429,8 +1429,8 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) user_cert_size += certlist->SignatureListSize;; user_cert = ReallocatePool(user_cert, original, user_cert_size); - memcpy(user_cert + original, pointer, - certlist->SignatureListSize); + CopyMem(user_cert + original, pointer, + certlist->SignatureListSize); } } FreePool(data);