From badd40f9d41c92bf54e3ef8d77f35ded05b7c918 Mon Sep 17 00:00:00 2001
From: tye1 <tye1@6f19259b-4bc3-4df7-8a09-765794883524>
Date: Tue, 8 May 2012 02:53:49 +0000
Subject: [PATCH] Removes redundant code and adds data size check for
 certificate data in DxeImageVerificationLib.

Signed-off by: Ye Ting <ting.ye@intel.com>
Reviewed-by: Dong Eric <yong.dong@intel.com>


git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13291 6f19259b-4bc3-4df7-8a09-765794883524
---
 .../DxeImageVerificationLib.c                 | 39 ++++++++++++-------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index a591970e45..3e0bbe1ee4 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -251,8 +251,6 @@ HashPeImage (
   EFI_IMAGE_SECTION_HEADER  *SectionHeader;
   UINTN                     Index;
   UINTN                     Pos;
-  UINTN                     SumOfSectionBytes;
-  EFI_IMAGE_SECTION_HEADER  *SectionCache;
   UINT32                    CertSize;
   UINT32                    NumberOfRvaAndSizes;
 
@@ -433,11 +431,6 @@ HashPeImage (
                mNtHeader.Pe32->FileHeader.SizeOfOptionalHeader
                );
 
-  SectionCache = Section;
-  for (Index = 0, SumOfSectionBytes = 0; Index < mNtHeader.Pe32->FileHeader.NumberOfSections; Index++, SectionCache++) {
-    SumOfSectionBytes += SectionCache->SizeOfRawData;
-  }
-
   //
   // 11. Build a temporary table of pointers to all the IMAGE_SECTION_HEADER
   //     structures in the image. The 'NumberOfSections' field of the image
@@ -557,6 +550,10 @@ HashPeImageByType (
 
   PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *) (mImageBase + mSecDataDir->VirtualAddress);
 
+  if (PkcsCertData->Hdr.dwLength < sizeof (WIN_CERTIFICATE_EFI_PKCS) + 32) {
+    return EFI_UNSUPPORTED;
+  }
+
   for (Index = 0; Index < HASHALG_MAX; Index++) {
     //
     // Check the Hash algorithm in PE/COFF Authenticode.
@@ -577,6 +574,10 @@ HashPeImageByType (
       continue;
     }
 
+    if (PkcsCertData->Hdr.dwLength < sizeof (WIN_CERTIFICATE_EFI_PKCS) + 32 + mHash[Index].OidLength) {
+      return EFI_UNSUPPORTED;
+    }
+
     if (CompareMem (PkcsCertData->CertData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {
       break;
     }
@@ -1214,6 +1215,7 @@ DxeImageVerificationHandler (
   UINT8                                *SecureBootEnable;
   PE_COFF_LOADER_IMAGE_CONTEXT         ImageContext;
   UINT32                               NumberOfRvaAndSizes;
+  UINT32                               CertSize;
 
   if (File == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -1321,7 +1323,9 @@ DxeImageVerificationHandler (
     goto Done;
   }
 
-  DosHdr      = (EFI_IMAGE_DOS_HEADER *) mImageBase;
+  Status = EFI_ACCESS_DENIED;
+
+  DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase;
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
     //
     // DOS image header is present,
@@ -1339,7 +1343,6 @@ DxeImageVerificationHandler (
     //
     // It is not a valid Pe/Coff file.
     //
-    Status = EFI_ACCESS_DENIED;
     goto Done;
   }
 
@@ -1374,8 +1377,6 @@ DxeImageVerificationHandler (
       //
       // Image Hash is in forbidden database (DBX).
       //
-      Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
-      Status = EFI_ACCESS_DENIED;
       goto Done;
     }
 
@@ -1389,8 +1390,6 @@ DxeImageVerificationHandler (
     //
     // Image Hash is not found in both forbidden and allowed database.
     //
-    Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
-    Status = EFI_ACCESS_DENIED;
     goto Done;
   }
 
@@ -1399,9 +1398,20 @@ DxeImageVerificationHandler (
   //
   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + mSecDataDir->VirtualAddress);
 
+  CertSize = sizeof (WIN_CERTIFICATE);
+
+  if ((mSecDataDir->Size <= CertSize) || (mSecDataDir->Size < WinCertificate->dwLength)) {
+    goto Done;
+  }
+
   switch (WinCertificate->wCertificateType) {
 
   case WIN_CERT_TYPE_EFI_GUID:
+    CertSize = sizeof (WIN_CERTIFICATE_UEFI_GUID) + sizeof (EFI_CERT_BLOCK_RSA_2048_SHA256) - sizeof (UINT8);
+    if (WinCertificate->dwLength < CertSize) {
+      goto Done;
+    }
+
     //
     // Verify UEFI GUID type.
     //
@@ -1416,7 +1426,7 @@ DxeImageVerificationHandler (
     //
     // Verify Pkcs signed data type.
     //
-    Status    = HashPeImageByType();
+    Status = HashPeImageByType();
     if (EFI_ERROR (Status)) {
       goto Done;
     }
@@ -1435,7 +1445,6 @@ DxeImageVerificationHandler (
     break;
 
   default:
-    Status = EFI_ACCESS_DENIED;
     goto Done;
   }
   //
-- 
2.39.2