From 11b5c5f9836b7a2aa4ca8c0e41efb21132038f49 Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Fri, 3 Nov 2023 10:08:20 -0700 Subject: [PATCH] Delete Old Stack Cookie Library and Supporting Code Description Delete the old stack cookie logic. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested on GCC and VS builds of Q35 Integration Instructions N/A --- MdePkg/Include/IndustryStandard/PeImage.h | 28 ---- MdePkg/Include/Library/BaseBinSecurityLib.h | 39 ----- MdePkg/Include/Library/PeCoffLib.h | 31 ---- .../BaseBinSecurityLibNull.c | 40 ----- .../BaseBinSecurityLibNull.inf | 24 --- .../BaseBinSecurityLibRng.c | 88 ---------- .../BaseBinSecurityLibRng.inf | 38 ----- .../X64/GSHandlerCheck.asm | 18 -- .../X64/report_rangecheckfailure.asm | 18 -- .../X64/security_check_cookie.asm | 24 --- MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 155 ------------------ 11 files changed, 503 deletions(-) delete mode 100644 MdePkg/Include/Library/BaseBinSecurityLib.h delete mode 100644 MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.c delete mode 100644 MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.inf delete mode 100644 MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.c delete mode 100644 MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf delete mode 100644 MdePkg/Library/BaseBinSecurityLibRng/X64/GSHandlerCheck.asm delete mode 100644 MdePkg/Library/BaseBinSecurityLibRng/X64/report_rangecheckfailure.asm delete mode 100644 MdePkg/Library/BaseBinSecurityLibRng/X64/security_check_cookie.asm diff --git a/MdePkg/Include/IndustryStandard/PeImage.h b/MdePkg/Include/IndustryStandard/PeImage.h index 1230eece1e7..18ac362d8a4 100644 --- a/MdePkg/Include/IndustryStandard/PeImage.h +++ b/MdePkg/Include/IndustryStandard/PeImage.h @@ -134,34 +134,6 @@ typedef struct { UINT32 Size; } EFI_IMAGE_DATA_DIRECTORY; -// MS_CHANGE_? - Something to do with Stack Cookies? -// -// Load Config Directory from PeCoff -// -typedef struct { - UINTN Size; - UINTN TimeDateStamp; - UINT8 MajorVersion; - UINT8 MinorVersion; - UINTN GlobalFlagsClear; - UINTN GlobalFlagsSet; - UINTN CriticalSectionDefaultTimeout; - UINTN DeCommitFreeBlockThreshold; - UINTN DeCommitTotalFreeThreshold; - UINTN LockPrefixTable; // VA - UINTN MaximumAllocationSize; - UINTN VirtualMemoryThreshold; - UINTN ProcessHeapFlags; - UINTN ProcessAffinityMask; - UINT8 CSDVersion; - UINT8 Reserved1; - UINTN EditList; // VA - UINTN SecurityCookie; - VOID **SEHandlerTable; - UINTN SEHandlerCount; -} EFI_IMAGE_LOAD_CONFIG_DIRECTORY; -// END - // // Directory Entries // diff --git a/MdePkg/Include/Library/BaseBinSecurityLib.h b/MdePkg/Include/Library/BaseBinSecurityLib.h deleted file mode 100644 index c631724d560..00000000000 --- a/MdePkg/Include/Library/BaseBinSecurityLib.h +++ /dev/null @@ -1,39 +0,0 @@ -/** @file -- BaseBinSecurityLib.h - -MS_CHANGE_? - -Copyright (c) Microsoft Corporation. All rights reserved. -SPDX-License-Identifier: BSD-2-Clause-Patent - -**/ - -#ifndef _BASE_BIN_SECURITY_LIB_H_ -#define _BASE_BIN_SECURITY_LIB_H_ - -/** -Initialize the security cookie present in this module - -This function uses the function below to initialize the __security_cookie -value that's inserted by the compiler when the security cookie compiler -flag is not disabled. - -**/ -VOID -EFIAPI -InitializeSecurityCookie ( - ); - -/** -Initialize a security cookie - -This function initializes the security cookie of which the address is passed. - -@param SecurityCookieAddress The address of the cookie to be initialized -**/ -VOID -EFIAPI -InitializeSecurityCookieAddress ( - UINT64 *SecurityCookieAddress - ); - -#endif // _BASE_BIN_SECURITY_LIB_H_ diff --git a/MdePkg/Include/Library/PeCoffLib.h b/MdePkg/Include/Library/PeCoffLib.h index b5af44958e1..1f1daa8d033 100644 --- a/MdePkg/Include/Library/PeCoffLib.h +++ b/MdePkg/Include/Library/PeCoffLib.h @@ -230,37 +230,6 @@ PeCoffLoaderGetImageInfo ( IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext ); -// MS_CHANGE_? START - -/** -Retrieves the address of the SecurityCookie from the PE/COFF image. - -This function locations the EFI_IMAGE_LOAD_CONFIG_DIRECTORY in the PE/COFF image -and gets the SecurityCookie field from this structure. This structure will only -exist in binaries built with the VS2013/VS2015 compilers without the /GS- flag. - -NOTE: Only X64 binaries are supported at the moment - -@param ImageContext The pointer to the image context structure that - describes the PE/COFF image that needs to be - examined by this function. -@param SecurityCookieAddress The pointer that receives the address of the - security cookie upon successful execution of this function - -@retval RETURN_SUCCESS The information on the PE/COFF image was collected. -@retval RETURN_INVALID_PARAMETER ImageContext is NULL. -@retval RETURN_UNSUPPORTED The PE/COFF image is not supported. - -**/ -RETURN_STATUS -EFIAPI -PeCoffLoaderGetSecurityCookieAddress ( - IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext, - OUT UINT64 **SecurityCookieAddress - ); - -// END - /** Applies relocation fixups to a PE/COFF image that was loaded with PeCoffLoaderLoadImage(). diff --git a/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.c b/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.c deleted file mode 100644 index 7af3f35de6b..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.c +++ /dev/null @@ -1,40 +0,0 @@ -/** @file -- BaseBinSecurityLibNull.c - -MS_CHANGE_? - -Copyright (c) Microsoft Corporation. All rights reserved. -SPDX-License-Identifier: BSD-2-Clause-Patent - -**/ - -#include - -/** -Initialize the security cookie present in this module - -This function uses the function below to initialize the __security_cookie -value that's inserted by the compiler when the security cookie compiler -flag is not disabled. - -**/ -VOID -EFIAPI -InitializeSecurityCookie ( - ) -{ -} - -/** -Initialize a security cookie - -This function initializes the security cookie of which the address is passed. - -@param SecurityCookieAddress The address of the cookie to be initialized -**/ -VOID -EFIAPI -InitializeSecurityCookieAddress ( - UINT64 *SecurityCookieAddress - ) -{ -} diff --git a/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.inf b/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.inf deleted file mode 100644 index 8a6ad4b7ff3..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibNull/BaseBinSecurityLibNull.inf +++ /dev/null @@ -1,24 +0,0 @@ -## @file BaseBinSecurityLibNull.inf -# -# MS_CHANGE_? -# -## -# Copyright (c) Microsoft Corporation. All rights reserved. -# SPDX-License-Identifier: BSD-2-Clause-Patent -## - - -[Defines] - INF_VERSION = 0x00010005 - BASE_NAME = BaseBinSecurityLibNull - FILE_GUID = f6ef2763-ca3b-4c6f-a931-2a48de3ce352 - MODULE_TYPE = BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = BaseBinSecurityLib - -[Sources] - BaseBinSecurityLibNull.c - -[Packages] - MdePkg/MdePkg.dec - diff --git a/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.c b/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.c deleted file mode 100644 index 6a66b61c5cb..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.c +++ /dev/null @@ -1,88 +0,0 @@ -/** @file -- BaseBinSecurityLibRng.c - -MS_CHANGE_? - -Copyright (c) Microsoft Corporation. All rights reserved. -SPDX-License-Identifier: BSD-2-Clause-Patent - -**/ - -#include -#include -#include -#include -#include -#include -#include // GetRandomNumber64() - -// -// Initialize the initial security cookie to zero -// in case we don't initialize it by the loader -// somewhere -// -UINT64 __security_cookie = 0; - -// -// Define the _load_config_used symbol for -// the linker to export so that the loader -// can find the location of the security cookie -// with in the binary later -// -__declspec(selectany) -EFI_IMAGE_LOAD_CONFIG_DIRECTORY _load_config_used = { - sizeof (EFI_IMAGE_LOAD_CONFIG_DIRECTORY), - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - (UINT64)&__security_cookie, - 0, - 0 -}; - -/** -Initialize the security cookie present in this module - -This function uses the function below to initialize the __security_cookie -value that's inserted by the compiler when the security cookie compiler -flag is not disabled. - -**/ -VOID -EFIAPI -InitializeSecurityCookie ( - ) -{ - UINT64 RandomValue = 0; - - InitializeSecurityCookieAddress (&RandomValue); - __security_cookie = RandomValue; -} - -/** -Initialize a security cookie - -This function initializes the security cookie of which the address is passed. - -@param SecurityCookieAddress The address of the cookie to be initialized -**/ -VOID -EFIAPI -InitializeSecurityCookieAddress ( - UINT64 *SecurityCookieAddress - ) -{ - GetRandomNumber64 (SecurityCookieAddress); -} diff --git a/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf b/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf deleted file mode 100644 index 9a8d3baa014..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibRng/BaseBinSecurityLibRng.inf +++ /dev/null @@ -1,38 +0,0 @@ -## @file BaseBinSecurityLibRng.inf -# -# MS_CHANGE_? -# -## -# Copyright (c) Microsoft Corporation. All rights reserved. -# SPDX-License-Identifier: BSD-2-Clause-Patent -## - - -[Defines] - INF_VERSION = 0x00010005 - BASE_NAME = BaseBinSecurityLibRng - FILE_GUID = 495b10c8-7148-4b62-92b0-7cf4551df83d - MODULE_TYPE = BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = BaseBinSecurityLib - -[Sources] - BaseBinSecurityLibRng.c - -[Sources.X64] - X64/report_rangecheckfailure.asm - X64/GSHandlerCheck.asm - X64/security_check_cookie.asm - -[Packages] - MdePkg/MdePkg.dec - -[LibraryClasses] - BaseMemoryLib - DebugLib - BaseLib - RngLib - PcdLib - -[FixedPcd] - gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector diff --git a/MdePkg/Library/BaseBinSecurityLibRng/X64/GSHandlerCheck.asm b/MdePkg/Library/BaseBinSecurityLibRng/X64/GSHandlerCheck.asm deleted file mode 100644 index bd1de566f4d..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibRng/X64/GSHandlerCheck.asm +++ /dev/null @@ -1,18 +0,0 @@ -;------------------------------------------------------------------------------ -; GSHandlerCheck.asm -; MS_CHANGE_? -;------------------------------------------------------------------------------ - .code - -;------------------------------------------------------------------------------ -; VOID -; EFIAPI -; __GSHandlerCheck (VOID); -;------------------------------------------------------------------------------ -__GSHandlerCheck PROC PUBLIC - ret ; this GS handler is for checking the stack cookie during SEH or - ; EH exceptions since UEFI doesn't support these we can just do - ; nothing here -__GSHandlerCheck ENDP - - END \ No newline at end of file diff --git a/MdePkg/Library/BaseBinSecurityLibRng/X64/report_rangecheckfailure.asm b/MdePkg/Library/BaseBinSecurityLibRng/X64/report_rangecheckfailure.asm deleted file mode 100644 index 2546ba75a55..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibRng/X64/report_rangecheckfailure.asm +++ /dev/null @@ -1,18 +0,0 @@ -;------------------------------------------------------------------------------ -; report_rangecheckfailure.asm -; MS_CHANGE_? -;------------------------------------------------------------------------------ - .code - -;------------------------------------------------------------------------------ -; VOID -; EFIAPI -; __report_rangecheckfailure (VOID); -;------------------------------------------------------------------------------ - -__report_rangecheckfailure PROC PUBLIC - int 3 - ret -__report_rangecheckfailure ENDP - - END diff --git a/MdePkg/Library/BaseBinSecurityLibRng/X64/security_check_cookie.asm b/MdePkg/Library/BaseBinSecurityLibRng/X64/security_check_cookie.asm deleted file mode 100644 index cbc2cbeb0bd..00000000000 --- a/MdePkg/Library/BaseBinSecurityLibRng/X64/security_check_cookie.asm +++ /dev/null @@ -1,24 +0,0 @@ -;------------------------------------------------------------------------------ -; security_check_cookie.asm -; MS_CHANGE_? -;------------------------------------------------------------------------------ - EXTRN __security_cookie:QWORD - .code - -;------------------------------------------------------------------------------ -; VOID -; EFIAPI -; __security_check_cookie ( -; IN UINTN CheckValue); -;------------------------------------------------------------------------------ -__security_check_cookie PROC PUBLIC - cmp rcx, __security_cookie - jne __security_check_cookie_Failure - ret - -__security_check_cookie_Failure: - int FixedPcdGet8 (PcdStackCookieExceptionVector) - ret -__security_check_cookie ENDP - - END diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c index dabdf7187e2..c3c3ec0f602 100644 --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c @@ -42,40 +42,6 @@ PeCoffLoaderAdjustOffsetForTeImage ( SectionHeader->PointerToRawData -= TeStrippedOffset; } -// MU_CHANGE [BEGIN] - Keep this function while it's used for the stack cookies. - -/** - Retrieves the magic value from the PE/COFF header. - - @param Hdr The buffer in which to return the PE32, PE32+, or TE header. - - @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32 - @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+ - -**/ -UINT16 -PeCoffLoaderGetPeHeaderMagicValue ( - IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr - ) -{ - // - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - // - if ((Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64) && (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC)) { - return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC; - } - - // - // Return the magic value from the PC/COFF Optional Header - // - return Hdr.Pe32->OptionalHeader.Magic; -} - -// MU_CHANGE [END] - Keep this function while it's used for the stack cookies. - /** Retrieves the PE or TE Header from a PE/COFF or TE image. @@ -930,127 +896,6 @@ PeCoffLoaderImageAddress ( return (CHAR8 *)((UINTN)ImageContext->ImageAddress + Address - TeStrippedOffset); } -// MS_CHANGE_? - -/** -Retrieves the address of the SecurityCookie from the PE/COFF image. - -This function locations the EFI_IMAGE_LOAD_CONFIG_DIRECTORY in the PE/COFF image -and gets the SecurityCookie field from this structure. This structure will only -exist in binaries built with the VS2013/VS2015 compilers without the /GS- flag. - -NOTE: Only X64 binaries are supported at the moment - -@param ImageContext The pointer to the image context structure that -describes the PE/COFF image that needs to be -examined by this function. -@param SecurityCookieAddress The pointer that receives the address of the -security cookie upon successful execution of this function - -@retval RETURN_SUCCESS The information on the PE/COFF image was collected. -@retval RETURN_INVALID_PARAMETER ImageContext is NULL. -@retval RETURN_UNSUPPORTED The PE/COFF image is not supported. - -**/ -RETURN_STATUS -EFIAPI -PeCoffLoaderGetSecurityCookieAddress ( - IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext, - OUT UINT64 **SecurityCookieAddress - ) -{ - EFI_IMAGE_LOAD_CONFIG_DIRECTORY *LoadConfigDataDirectory; - EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr; - EFI_IMAGE_DATA_DIRECTORY *DataDirectory; - UINT64 Adjust; - PHYSICAL_ADDRESS BaseAddress; - UINT32 NumberOfRvaAndSizes; - UINT16 Magic; - - *SecurityCookieAddress = NULL; - - if (ImageContext == NULL) { - return RETURN_INVALID_PARAMETER; - } - - // - // If the destination address is not 0, use that rather than the - // image address as the relocation target. - // - if (ImageContext->DestinationAddress != 0) { - BaseAddress = ImageContext->DestinationAddress; - } else { - BaseAddress = ImageContext->ImageAddress; - } - - if ((ImageContext->Machine != IMAGE_FILE_MACHINE_X64) || ImageContext->IsTeImage) { - // DEBUG((DEBUG_INFO | DEBUG_LOAD, "SecurityCookie is only supported on X64 binaries\n")); - } else { - Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + ImageContext->PeCoffHeaderOffset); - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr); - - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) { - // - // Use PE32 offset - // - Adjust = (UINT64)BaseAddress - Hdr.Pe32->OptionalHeader.ImageBase; - if (Adjust != 0) { - Hdr.Pe32->OptionalHeader.ImageBase = (UINT32)BaseAddress; - } - - NumberOfRvaAndSizes = Hdr.Pe32->OptionalHeader.NumberOfRvaAndSizes; - DataDirectory = &Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG]; - } else { - // - // Use PE32+ offset - // - Adjust = (UINT64)BaseAddress - Hdr.Pe32Plus->OptionalHeader.ImageBase; - if (Adjust != 0) { - Hdr.Pe32Plus->OptionalHeader.ImageBase = (UINT64)BaseAddress; - } - - NumberOfRvaAndSizes = Hdr.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes; - DataDirectory = &Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG]; - } - - // - // Find the relocation block - // Per the PE/COFF spec, you can't assume that a given data directory - // is present in the image. You have to check the NumberOfRvaAndSizes in - // the optional header to verify a desired directory entry is there. - // - if ((NumberOfRvaAndSizes < EFI_IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG)) { - DataDirectory = NULL; - } - - if (DataDirectory != NULL) { - if (DataDirectory->Size == sizeof (EFI_IMAGE_LOAD_CONFIG_DIRECTORY)) { - LoadConfigDataDirectory = (EFI_IMAGE_LOAD_CONFIG_DIRECTORY *)PeCoffLoaderImageAddress (ImageContext, DataDirectory->VirtualAddress, 0); - if (LoadConfigDataDirectory != NULL) { - if ((LoadConfigDataDirectory->SecurityCookie > BaseAddress) && - (LoadConfigDataDirectory->SecurityCookie + sizeof (UINT64) < BaseAddress + ImageContext->ImageSize)) - { - *SecurityCookieAddress = (UINT64 *)LoadConfigDataDirectory->SecurityCookie; - return RETURN_SUCCESS; - } else { - DEBUG ((DEBUG_INFO | DEBUG_LOAD, "SecurityCookieAddress is invalid\n")); - } - } else { - DEBUG ((DEBUG_INFO | DEBUG_LOAD, "LoadConfigDataDirectory is NULL\n")); - } - } else { - DEBUG ((DEBUG_INFO | DEBUG_LOAD, "DataDirectory->Size != sizeof(EFI_IMAGE_LOAD_CONFIG_DIRECTORY)\n")); - } - } else { - DEBUG ((DEBUG_INFO | DEBUG_LOAD, "DataDirectory is NULL\n")); - } - } - - return RETURN_UNSUPPORTED; -} - -// END - /** Applies relocation fixups to a PE/COFF image that was loaded with PeCoffLoaderLoadImage().