It's strange to me that ARM's MM env still allows modifying HOBs. Thanks, Ray > -----Original Message----- > From: Guo, Gua <gua....@intel.com> > Sent: Friday, January 12, 2024 10:25 AM > To: devel@edk2.groups.io > Cc: Guo, Gua <gua....@intel.com>; Marc Beatove <mbeat...@google.com>; > Ard Biesheuvel <ardb+tianoc...@kernel.org>; Sami Mujawar > <sami.muja...@arm.com>; Ni, Ray <ray...@intel.com>; Mathews, John > <john.math...@intel.com>; Gerd Hoffmann <kra...@redhat.com> > Subject: [PATCH v3 2/4] StandaloneMmPkg/Hob: Integer Overflow in > CreateHob() > > From: Gua Guo <gua....@intel.com> > > 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 <mbeat...@google.com> > Reviewed-by: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Sami Mujawar <sami.muja...@arm.com> > Cc: Ray Ni <ray...@intel.com> > Cc: John Mathew <john.math...@intel.com> > Authored-by: Gerd Hoffmann <kra...@redhat.com> > Signed-off-by: Gua Guo <gua....@intel.com> > --- > .../Arm/StandaloneMmCoreHobLib.c | 35 +++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git > a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM > mCoreHobLib.c > b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM > mCoreHobLib.c > index 1550e1babc..59473e28fe 100644 > --- > a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM > mCoreHobLib.c > +++ > b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Arm/StandaloneM > mCoreHobLib.c > @@ -34,6 +34,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; > > @@ -89,6 +96,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; > > @@ -129,6 +140,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; > > @@ -167,6 +181,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; > > } > > @@ -226,6 +245,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; > > @@ -255,6 +278,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; > > @@ -282,6 +309,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; > > @@ -319,6 +350,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; > > -- > 2.39.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113699): https://edk2.groups.io/g/devel/message/113699 Mute This Topic: https://groups.io/mt/103675962/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
