diff --git a/backport-0001-CVE-2022-28737.patch b/backport-0001-CVE-2022-28737.patch new file mode 100644 index 0000000000000000000000000000000000000000..9c1018219a14b23b12991dad99d11d3c2bcd01a3 --- /dev/null +++ b/backport-0001-CVE-2022-28737.patch @@ -0,0 +1,62 @@ +From e99bdbb827a50cde019393d3ca1e89397db221a7 Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Tue, 3 May 2022 15:41:00 +0200 +Subject: [PATCH] pe: Fix a buffer overflow when SizeOfRawData > VirtualSize + +During image loading, the size of the destination buffer for the image +is determined by the SizeOfImage field in the optional header. The start +and end virtual addresses of each section, as determined by each section's +VirtualAddress and VirtualSize fields, are bounds checked against the +allocated buffer. However, the amount of data copied to the destination +buffer is determined by the section's SizeOfRawData filed. If this is +larger than the VirtualSize, then the copy can overflow the destination +buffer. + +Fix this by limiting the amount of data to copy to the section's +VirtualSize. In the case where a section has SizeOfRawData > VirtualSize, +the excess data is discarded. + +This fixes CVE-2022-28737 + +Signed-off-by: Chris Coulson +--- + pe.c | 15 +++++++++------ + 1 file changed, 9 insertions(+), 6 deletions(-) + +diff --git a/pe.c b/pe.c +index 5d0c6b0..1eb3f59 100644 +--- a/pe.c ++++ b/pe.c +@@ -1089,6 +1089,7 @@ handle_image (void *data, unsigned int datasize, + int i; + EFI_IMAGE_SECTION_HEADER *Section; + char *base, *end; ++ UINT32 size; + PE_COFF_LOADER_IMAGE_CONTEXT context; + unsigned int alignment, alloc_size; + int found_entry_point = 0; +@@ -1274,13 +1275,15 @@ handle_image (void *data, unsigned int datasize, + return EFI_UNSUPPORTED; + } + +- if (Section->SizeOfRawData > 0) +- CopyMem(base, data + Section->PointerToRawData, +- Section->SizeOfRawData); ++ size = Section->Misc.VirtualSize; ++ if (size > Section->SizeOfRawData) ++ size = Section->SizeOfRawData; + +- if (Section->SizeOfRawData < Section->Misc.VirtualSize) +- ZeroMem(base + Section->SizeOfRawData, +- Section->Misc.VirtualSize - Section->SizeOfRawData); ++ if (size > 0) ++ CopyMem(base, data + Section->PointerToRawData, size); ++ ++ if (size < Section->Misc.VirtualSize) ++ ZeroMem(base + size, Section->Misc.VirtualSize - size); + } + } + +-- +2.27.0 + diff --git a/backport-0002-CVE-2022-28737.patch b/backport-0002-CVE-2022-28737.patch new file mode 100644 index 0000000000000000000000000000000000000000..a0964e4a8fba2c3a923b0170f3f4d1dc73e688b2 --- /dev/null +++ b/backport-0002-CVE-2022-28737.patch @@ -0,0 +1,78 @@ +From 5a82d7973656c68f006aac1ed462e7bb37075d92 Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Tue, 3 May 2022 16:02:19 +0200 +Subject: [PATCH] pe: Perform image verification earlier when loading grub + +The second stage loader was being verified after loading it into +memory. As an additional hardening measure to avoid performing risky +memcpys using header fields from a potentially specially crafted image, +perform the verification before this so that it can be rejected earlier. + +Signed-off-by: Chris Coulson +--- + pe.c | 42 +++++++++++++++++++++++++----------------- + 1 file changed, 25 insertions(+), 17 deletions(-) + +diff --git a/pe.c b/pe.c +index 1eb3f59..1d120f2 100644 +--- a/pe.c ++++ b/pe.c +@@ -1106,7 +1106,31 @@ handle_image (void *data, unsigned int datasize, + } + + /* +- * We only need to verify the binary if we're in secure mode ++ * Perform the image verification before we start copying data around ++ * in order to load it. ++ */ ++ if (secure_mode ()) { ++ efi_status = verify_buffer(data, datasize, &context, sha256hash, ++ sha1hash); ++ ++ if (EFI_ERROR(efi_status)) { ++ if (verbose) ++ console_print(L"Verification failed: %r\n", efi_status); ++ else ++ console_error(L"Verification failed", efi_status); ++ return efi_status; ++ } else { ++ if (verbose) ++ console_print(L"Verification succeeded\n"); ++ } ++ } ++ ++ /* ++ * Calculate the hash for the TPM measurement. ++ * XXX: We're computing these twice in secure boot mode when the ++ * buffers already contain the previously computed hashes. Also, ++ * this is only useful for the TPM1.2 case. We should try to fix ++ * this in a follow-up. + */ + efi_status = generate_hash(data, datasize, &context, sha256hash, + sha1hash); +@@ -1287,22 +1311,6 @@ handle_image (void *data, unsigned int datasize, + } + } + +- if (secure_mode ()) { +- efi_status = verify_buffer(data, datasize, &context, sha256hash, +- sha1hash); +- +- if (EFI_ERROR(efi_status)) { +- if (verbose) +- console_print(L"Verification failed: %r\n", efi_status); +- else +- console_error(L"Verification failed", efi_status); +- return efi_status; +- } else { +- if (verbose) +- console_print(L"Verification succeeded\n"); +- } +- } +- + if (context.NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) { + perror(L"Image has no relocation entry\n"); + FreePool(buffer); +-- +2.27.0 + diff --git a/backport-shim-implement-SBAT-verification-for-the-shim_lock-p.patch b/backport-shim-implement-SBAT-verification-for-the-shim_lock-p.patch new file mode 100644 index 0000000000000000000000000000000000000000..31ba7cf73b98f06e0c6f6f8cad470b91d501f50f --- /dev/null +++ b/backport-shim-implement-SBAT-verification-for-the-shim_lock-p.patch @@ -0,0 +1,225 @@ +From a2da05fcb8972628bec08e4adfc13abbafc319ad Mon Sep 17 00:00:00 2001 +From: Chris Coulson +Date: Mon, 28 Feb 2022 21:29:16 +0000 +Subject: [PATCH] shim: implement SBAT verification for the shim_lock protocol + +This implements SBAT verification via the shim_lock protocol +by moving verification inside the existing verify_buffer() +function that is shared by both shim_verify() and handle_image(). + +The .sbat section is optional for code verified via the shim_lock +protocol, unlike for code that is verified and executed directly +by shim. For executables that don't have a .sbat section, +verification is skipped when using the protocol. + +A vendor can enforce SBAT verification for code verified via the +shim_lock protocol by revoking all pre-SBAT binaries via a dbx +update or by using vendor_dbx and then only signing binaries that +have a .sbat section from that point. + +Signed-off-by: Chris Coulson +--- + include/pe.h | 2 +- + pe.c | 46 +++++++-------------------------- + shim.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++--- + 3 files changed, 79 insertions(+), 42 deletions(-) + +diff --git a/include/pe.h b/include/pe.h +index 43727f5..b86e1b3 100644 +--- a/include/pe.h ++++ b/include/pe.h +@@ -15,7 +15,7 @@ read_header(void *data, unsigned int datasize, + PE_COFF_LOADER_IMAGE_CONTEXT *context); + + EFI_STATUS +-handle_sbat(char *SBATBase, size_t SBATSize); ++verify_sbat_section(char *SBATBase, size_t SBATSize); + + EFI_STATUS + handle_image (void *data, unsigned int datasize, +diff --git a/pe.c b/pe.c +index 92c2804..554e77c 100644 +--- a/pe.c ++++ b/pe.c +@@ -820,7 +820,7 @@ read_header(void *data, unsigned int datasize, + } + + EFI_STATUS +-handle_sbat(char *SBATBase, size_t SBATSize) ++verify_sbat_section(char *SBATBase, size_t SBATSize) + { + unsigned int i; + EFI_STATUS efi_status; +@@ -834,7 +834,12 @@ handle_sbat(char *SBATBase, size_t SBATSize) + + if (SBATBase == NULL || SBATSize == 0) { + dprint(L"No .sbat section data\n"); +- return EFI_SECURITY_VIOLATION; ++ /* ++ * SBAT is mandatory for binaries loaded by shim, but optional ++ * for binaries loaded outside of shim but verified via the ++ * protocol. ++ */ ++ return in_protocol ? EFI_SUCCESS : EFI_SECURITY_VIOLATION; + } + + sbat_size = SBATSize + 1; +@@ -980,9 +985,6 @@ handle_image (void *data, unsigned int datasize, + + EFI_IMAGE_SECTION_HEADER *RelocSection = NULL; + +- char *SBATBase = NULL; +- size_t SBATSize = 0; +- + /* + * Copy the executable's sections to their desired offsets + */ +@@ -1027,33 +1029,6 @@ handle_image (void *data, unsigned int datasize, + RelocBaseEnd == end) { + RelocSection = Section; + } +- } else if (CompareMem(Section->Name, ".sbat\0\0\0", 8) == 0) { +- if (SBATBase || SBATSize) { +- perror(L"Image has multiple SBAT sections\n"); +- return EFI_UNSUPPORTED; +- } +- +- if (Section->NumberOfRelocations != 0 || +- Section->PointerToRelocations != 0) { +- perror(L"SBAT section has relocations\n"); +- return EFI_UNSUPPORTED; +- } +- +- /* The virtual size corresponds to the size of the SBAT +- * metadata and isn't necessarily a multiple of the file +- * alignment. The on-disk size is a multiple of the file +- * alignment and is zero padded. Make sure that the +- * on-disk size is at least as large as virtual size, +- * and ignore the section if it isn't. */ +- if (Section->SizeOfRawData && +- Section->SizeOfRawData >= Section->Misc.VirtualSize && +- base && end) { +- SBATBase = base; +- /* +1 because of size vs last byte location */ +- SBATSize = end - base + 1; +- dprint(L"sbat section base:0x%lx size:0x%lx\n", +- SBATBase, SBATSize); +- } + } + + if (Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) { +@@ -1095,11 +1070,8 @@ handle_image (void *data, unsigned int datasize, + } + + if (secure_mode ()) { +- efi_status = handle_sbat(SBATBase, SBATSize); +- +- if (!EFI_ERROR(efi_status)) +- efi_status = verify_buffer(data, datasize, +- &context, sha256hash, sha1hash); ++ efi_status = verify_buffer(data, datasize, &context, sha256hash, ++ sha1hash); + + if (EFI_ERROR(efi_status)) { + if (verbose) +diff --git a/shim.c b/shim.c +index 604c0db..6d6b1e5 100644 +--- a/shim.c ++++ b/shim.c +@@ -559,9 +559,9 @@ verify_one_signature(WIN_CERTIFICATE_EFI_PKCS *sig, + * Check that the signature is valid and matches the binary + */ + EFI_STATUS +-verify_buffer (char *data, int datasize, +- PE_COFF_LOADER_IMAGE_CONTEXT *context, +- UINT8 *sha256hash, UINT8 *sha1hash) ++verify_buffer_authenticode (char *data, int datasize, ++ PE_COFF_LOADER_IMAGE_CONTEXT *context, ++ UINT8 *sha256hash, UINT8 *sha1hash) + { + EFI_STATUS ret_efi_status; + size_t size = datasize; +@@ -695,6 +695,71 @@ verify_buffer (char *data, int datasize, + return ret_efi_status; + } + ++/* ++ * Check that the binary is permitted to load by SBAT. ++ */ ++EFI_STATUS ++verify_buffer_sbat (char *data, int datasize, ++ PE_COFF_LOADER_IMAGE_CONTEXT *context) ++{ ++ int i; ++ EFI_IMAGE_SECTION_HEADER *Section; ++ char *SBATBase = NULL; ++ size_t SBATSize = 0; ++ ++ Section = context->FirstSection; ++ for (i = 0; i < context->NumberOfSections; i++, Section++) { ++ if (CompareMem(Section->Name, ".sbat\0\0\0", 8) != 0) ++ continue; ++ ++ if (SBATBase || SBATSize) { ++ perror(L"Image has multiple SBAT sections\n"); ++ return EFI_UNSUPPORTED; ++ } ++ ++ if (Section->NumberOfRelocations != 0 || ++ Section->PointerToRelocations != 0) { ++ perror(L"SBAT section has relocations\n"); ++ return EFI_UNSUPPORTED; ++ } ++ ++ /* The virtual size corresponds to the size of the SBAT ++ * metadata and isn't necessarily a multiple of the file ++ * alignment. The on-disk size is a multiple of the file ++ * alignment and is zero padded. Make sure that the ++ * on-disk size is at least as large as virtual size, ++ * and ignore the section if it isn't. */ ++ if (Section->SizeOfRawData && ++ Section->SizeOfRawData >= Section->Misc.VirtualSize) { ++ SBATBase = ImageAddress(data, datasize, ++ Section->PointerToRawData); ++ SBATSize = Section->SizeOfRawData; ++ dprint(L"sbat section base:0x%lx size:0x%lx\n", ++ SBATBase, SBATSize); ++ } ++ } ++ ++ return verify_sbat_section(SBATBase, SBATSize); ++} ++ ++/* ++ * Check that the signature is valid and matches the binary and that ++ * the binary is permitted to load by SBAT. ++ */ ++EFI_STATUS ++verify_buffer (char *data, int datasize, ++ PE_COFF_LOADER_IMAGE_CONTEXT *context, ++ UINT8 *sha256hash, UINT8 *sha1hash) ++{ ++ EFI_STATUS efi_status; ++ ++ efi_status = verify_buffer_sbat(data, datasize, context); ++ if (EFI_ERROR(efi_status)) ++ return efi_status; ++ ++ return verify_buffer_authenticode(data, datasize, context, sha256hash, sha1hash); ++} ++ + static int + should_use_fallback(EFI_HANDLE image_handle) + { +@@ -1542,7 +1607,7 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) + goto die; + } + +- efi_status = handle_sbat(sbat_start, sbat_end - sbat_start - 1); ++ efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1); + if (EFI_ERROR(efi_status)) { + perror(L"Verifiying shim SBAT data failed: %r\n", + efi_status); +-- +2.27.0 + diff --git a/shim.spec b/shim.spec index 0c5f5f03397365604663c1480e8394bfb3ae3caa..46e0cdf24309b7a649c33df6a0eee6edf5f3e024 100644 --- a/shim.spec +++ b/shim.spec @@ -22,7 +22,7 @@ Name: shim Version: 15.4 -Release: 3 +Release: 4 Summary: First-stage UEFI bootloader ExclusiveArch: x86_64 aarch64 License: BSD @@ -35,6 +35,9 @@ Patch0: backport-shim-another-attempt-to-fix-load-options-handling.patch Patch1: backport-arm-aa64-fix-the-size-of-.rela-sections.patch Patch2: backport-mok-relax-the-maximum-variable-size-check.patch Patch3: backport-mok-delete-the-existing-RT-variables-only-when-only_.patch +Patch4: backport-shim-implement-SBAT-verification-for-the-shim_lock-p.patch +Patch5: backport-0001-CVE-2022-28737.patch +Patch6: backport-0002-CVE-2022-28737.patch BuildRequires: elfutils-libelf-devel openssl-devel openssl git pesign gnu-efi gnu-efi-devel gcc Requires: dbxtool efi-filesystem mokutil @@ -141,6 +144,9 @@ cd .. /usr/src/debug/%{name}-%{version}-%{release}/* %changelog +* Wed Jul 27 2022 jinlun - 15.4-4 +- fix CVE-2022-28737 + * Tue Jul 5 2022 Hugel - 15.4-3 - fix shim occasionally crashes in _relocate() on AArch64