[edk2-devel] [PATCH V1 1/1] OvmfPkg/TdxHelperLib: Check the HobLength of EFI_HOB_GUID_TYPE

[sunceping]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4364

Currently, the length of type EFI_HOB_TYPE_GUID_EXTENSION is not checked
because it is variable length data. This might give a chance to an buffer
overflow issue.

Fix this by checking the HobLength of EFI_HOB_GUID_TYPE to make sure that
it is legal. In the meantime, the total size of TdHob is checked to ensure
the Hobs in TdHob would not overflow.

Cc: Erdem Aktas <erdemaktas at google.com>
Cc: James Bottomley <jejb at linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Min Xu <min.m.xu at intel.com>
Cc: Tom Lendacky <thomas.lendacky at amd.com>
Cc: Michael Roth <michael.roth at amd.com>
Signed-off-by: Sun Ceping <cepingx.sun at intel.com>
---
 OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c  | 20 ++++++++++++++++++-
 .../IntelTdx/TdxHelperLib/SecTdxHelperLib.inf |  1 +
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
index 3372cee2f720..818a6932cf66 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelper.c
@@ -566,11 +566,17 @@ ValidateHobList (
     BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED
   };
 
+  UINT32  TotalSize;
+  UINT32  TDHobSize;
+
   if (VmmHobList == NULL) {
     DEBUG ((DEBUG_ERROR, "HOB: HOB data pointer is NULL\n"));
     return FALSE;
   }
 
+  TotalSize = 0;
+  TDHobSize = (UINT32)FixedPcdGet32 (PcdOvmfSecGhcbSize);
+
   Hob.Raw = (UINT8 *)VmmHobList;
 
   //
@@ -587,6 +593,12 @@ ValidateHobList (
       return FALSE;
     }
 
+    TotalSize += Hob.Header->HobLength;
+    if (TotalSize > TDHobSize) {
+      DEBUG ((DEBUG_ERROR, "HOB: TD Hob Size was overflow. Totalsize is  0x%x\n", TotalSize));
+      return FALSE;
+    }
+
     switch (Hob.Header->HobType) {
       case EFI_HOB_TYPE_HANDOFF:
         if (Hob.Header->HobLength != sizeof (EFI_HOB_HANDOFF_INFO_TABLE)) {
@@ -651,8 +663,14 @@ ValidateHobList (
 
         break;
 
-      // EFI_HOB_GUID_TYPE is variable length data, so skip check
+      // EFI_HOB_GUID_TYPE is variable length data. The total size of the TdHob list is checked at the beginning of the loop.
+      // So we only need to check the min size of the HOB.
       case EFI_HOB_TYPE_GUID_EXTENSION:
+        if (Hob.Header->HobLength < sizeof (EFI_HOB_GUID_TYPE)) {
+          DEBUG ((DEBUG_ERROR, "HOB: Hob length is not less than corresponding hob structure. Type: 0x%04x\n", EFI_HOB_TYPE_GUID_EXTENSION));
+          return FALSE;
+        }
+
         break;
 
       case EFI_HOB_TYPE_FV:
diff --git a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
index d17b84c01f20..d5859588536b 100644
--- a/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
+++ b/OvmfPkg/IntelTdx/TdxHelperLib/SecTdxHelperLib.inf
@@ -46,6 +46,7 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
   gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
   gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101260): https://edk2.groups.io/g/devel/message/101260
Mute This Topic: https://groups.io/mt/97644707/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-