From aeaee8944f0eaacbf4cdf39279785b9ba4836bb6 Mon Sep 17 00:00:00 2001 From: Gua Guo Date: Thu, 11 Jan 2024 13:07:50 +0800 Subject: EmbeddedPkg/Hob: Integer Overflow in CreateHob() REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4166 Fix integer overflow in various CreateHob instances. Fixes: CVE-2022-36765 The CreateHob() function aligns the requested size to 8 performing the following operation: ``` HobLength = (UINT16)((HobLength + 0x7) & (~0x7)); ``` No checks are performed to ensure this value doesn't overflow, and could lead to CreateHob() returning a smaller HOB than requested, which could lead to OOB HOB accesses. Reported-by: Marc Beatove Cc: Leif Lindholm Reviewed-by: Ard Biesheuvel Cc: Abner Chang Cc: John Mathew Authored-by: Gerd Hoffmann Signed-off-by: Gua Guo --- EmbeddedPkg/Library/PrePiHobLib/Hob.c | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c index 8eb175a..cbc3515 100644 --- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c +++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c @@ -110,6 +110,13 @@ CreateHob ( HandOffHob = GetHobList (); + // + // Check Length to avoid data overflow. + // + if (HobLength > MAX_UINT16 - 0x7) { + return NULL; + } + HobLength = (UINT16)((HobLength + 0x7) & (~0x7)); FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob->EfiFreeMemoryBottom; @@ -160,6 +167,9 @@ BuildResourceDescriptorHob ( Hob = CreateHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, sizeof (EFI_HOB_RESOURCE_DESCRIPTOR)); ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } Hob->ResourceType = ResourceType; Hob->ResourceAttribute = ResourceAttribute; @@ -401,6 +411,10 @@ BuildModuleHob ( ); Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_MODULE)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } CopyGuid (&(Hob->MemoryAllocationHeader.Name), &gEfiHobMemoryAllocModuleGuid); Hob->MemoryAllocationHeader.MemoryBaseAddress = MemoryAllocationModule; @@ -449,6 +463,11 @@ BuildGuidHob ( ASSERT (DataLength <= (0xffff - sizeof (EFI_HOB_GUID_TYPE))); Hob = CreateHob (EFI_HOB_TYPE_GUID_EXTENSION, (UINT16)(sizeof (EFI_HOB_GUID_TYPE) + DataLength)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return NULL; + } + CopyGuid (&Hob->Name, Guid); return Hob + 1; } @@ -512,6 +531,10 @@ BuildFvHob ( EFI_HOB_FIRMWARE_VOLUME *Hob; Hob = CreateHob (EFI_HOB_TYPE_FV, sizeof (EFI_HOB_FIRMWARE_VOLUME)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } Hob->BaseAddress = BaseAddress; Hob->Length = Length; @@ -543,6 +566,10 @@ BuildFv2Hob ( EFI_HOB_FIRMWARE_VOLUME2 *Hob; Hob = CreateHob (EFI_HOB_TYPE_FV2, sizeof (EFI_HOB_FIRMWARE_VOLUME2)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } Hob->BaseAddress = BaseAddress; Hob->Length = Length; @@ -584,6 +611,10 @@ BuildFv3Hob ( EFI_HOB_FIRMWARE_VOLUME3 *Hob; Hob = CreateHob (EFI_HOB_TYPE_FV3, sizeof (EFI_HOB_FIRMWARE_VOLUME3)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } Hob->BaseAddress = BaseAddress; Hob->Length = Length; @@ -639,6 +670,10 @@ BuildCpuHob ( EFI_HOB_CPU *Hob; Hob = CreateHob (EFI_HOB_TYPE_CPU, sizeof (EFI_HOB_CPU)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } Hob->SizeOfMemorySpace = SizeOfMemorySpace; Hob->SizeOfIoSpace = SizeOfIoSpace; @@ -676,6 +711,10 @@ BuildStackHob ( ); Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION_STACK)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } CopyGuid (&(Hob->AllocDescriptor.Name), &gEfiHobMemoryAllocStackGuid); Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress; @@ -756,6 +795,10 @@ BuildMemoryAllocationHob ( ); Hob = CreateHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, sizeof (EFI_HOB_MEMORY_ALLOCATION)); + ASSERT (Hob != NULL); + if (Hob == NULL) { + return; + } ZeroMem (&(Hob->AllocDescriptor.Name), sizeof (EFI_GUID)); Hob->AllocDescriptor.MemoryBaseAddress = BaseAddress; -- cgit v1.1