Enhances PE image hash algorithm in DxeImageVerificationLib and DxeTpmMeasureBootLib.

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



git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13228 6f19259b-4bc3-4df7-8a09-765794883524

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c

@@ -253,6 +253,8 @@ HashPeImage (
   UINTN                     Pos;
   UINTN                     SumOfSectionBytes;
   EFI_IMAGE_SECTION_HEADER  *SectionCache;
+  UINT32                    CertSize;
+  UINT32                    NumberOfRvaAndSizes;
 
   HashCtx       = NULL;
   SectionHeader = NULL;
@@ -292,6 +294,7 @@ HashPeImage (
   if (!Status) {
     goto Done;
   }
+
   //
   // Measuring PE/COFF Image Header;
   // But CheckSum field and SECURITY data directory (certificate) are excluded
@@ -307,11 +310,13 @@ HashPeImage (
     // Use PE32 offset.
     //
     HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - HashBase);
+    NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
   } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
     //
     // Use PE32+ offset.
     //
     HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.CheckSum) - HashBase);
+    NumberOfRvaAndSizes = mNtHeader.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes;
   } else {
     //
     // Invalid header magic number.
@@ -324,9 +329,37 @@ HashPeImage (
   if (!Status) {
     goto Done;
   }
+
   //
   // 5.  Skip over the image checksum (it occupies a single ULONG).
-  // 6.  Get the address of the beginning of the Cert Directory.
+  //
+  if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
+    //
+    // 6.  Since there is no Cert Directory in optional header, hash everything
+    //     from the end of the checksum to the end of image header.
+    //
+    if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+      //
+      // Use PE32 offset.
+      //
+      HashBase = (UINT8 *) &mNtHeader.Pe32->OptionalHeader.CheckSum + sizeof (UINT32);
+      HashSize = mNtHeader.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - mImageBase);
+    } else {
+      //
+      // Use PE32+ offset.
+      //
+      HashBase = (UINT8 *) &mNtHeader.Pe32Plus->OptionalHeader.CheckSum + sizeof (UINT32);
+      HashSize = mNtHeader.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - mImageBase);
+    }
+
+    if (HashSize != 0) {
+      Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
+      if (!Status) {
+        goto Done;
+      }
+    }
+  } else {
+    //
     // 7.  Hash everything from the end of the checksum to the start of the Cert Directory.
     //
     if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
@@ -343,10 +376,13 @@ HashPeImage (
       HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - HashBase);
     }
 
+    if (HashSize != 0) {
       Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
       if (!Status) {
         goto Done;
       }
+    }
+
     //
     // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
     // 9.  Hash everything from the end of the Cert Directory to the end of image header.
@@ -356,19 +392,23 @@ HashPeImage (
       // Use PE32 offset
       //
       HashBase = (UINT8 *) &mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = mNtHeader.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) ((UINT8 *) (&mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - mImageBase);
+      HashSize = mNtHeader.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - mImageBase);
     } else {
       //
       // Use PE32+ offset.
       //
       HashBase = (UINT8 *) &mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = mNtHeader.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - mImageBase);
+      HashSize = mNtHeader.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - mImageBase);
     }
 
+    if (HashSize != 0) {
       Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
       if (!Status) {
         goto Done;
       }
+    }    
+  }
+
   //
   // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
   //
@@ -398,15 +438,6 @@ HashPeImage (
     SumOfSectionBytes += SectionCache->SizeOfRawData;
   }
 
-  //
-  // Sanity check for file corruption. Sections raw data size should be smaller
-  // than Image Size.
-  //
-  if (SumOfSectionBytes >= mImageSize) {
-    Status = FALSE;
-    goto Done;
-  }
-
   //
   // 11. Build a temporary table of pointers to all the IMAGE_SECTION_HEADER
   //     structures in the image. The 'NumberOfSections' field of the image
@@ -465,37 +496,36 @@ HashPeImage (
   //
   if (mImageSize > SumOfBytesHashed) {
     HashBase = mImageBase + SumOfBytesHashed;
+
+    if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
+      CertSize = 0;
+    } else {
       if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-      if (mImageSize - SumOfBytesHashed < mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size) {
-        Status = FALSE;
-        goto Done;
-      }
         //
         // Use PE32 offset.
         //
-      HashSize = (UINTN)(
+        CertSize = mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
-                 mImageSize -
-                 mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size -
-                 SumOfBytesHashed);
       } else {
-      if (mImageSize - SumOfBytesHashed < mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size) {
-        Status = FALSE;
-        goto Done;
-      }
         //
         // Use PE32+ offset.
         //
-      HashSize = (UINTN)(
+        CertSize = mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
-                 mImageSize -
-                 mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size -
-                 SumOfBytesHashed);
       }
+    }
+
+    if (mImageSize > CertSize + SumOfBytesHashed) {
+      HashSize = (UINTN) (mImageSize - CertSize - SumOfBytesHashed);
 
       Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
       if (!Status) {
         goto Done;
       }
+    } else if (mImageSize < CertSize + SumOfBytesHashed) {
+      Status = FALSE;
+      goto Done;
     }
+  }
+
   Status  = mHash[HashAlg].HashFinal(HashCtx, mImageDigest);
 
 Done:
@@ -1183,6 +1213,7 @@ DxeImageVerificationHandler (
   UINT32                               Policy;
   UINT8                                *SecureBootEnable;
   PE_COFF_LOADER_IMAGE_CONTEXT         ImageContext;
+  UINT32                               NumberOfRvaAndSizes;
 
   if (File == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -1243,6 +1274,8 @@ DxeImageVerificationHandler (
     return EFI_SUCCESS;
   }
 
+  FreePool (SecureBootEnable);
+
   SetupMode = GetEfiGlobalVariable (EFI_SETUP_MODE_NAME);
 
   //
@@ -1260,13 +1293,16 @@ DxeImageVerificationHandler (
     FreePool (SetupMode);
     return EFI_SUCCESS;
   }
+
+  FreePool (SetupMode);
+
   //
   // Read the Dos header.
   //
   if (FileBuffer == NULL) {
-    FreePool (SetupMode);
     return EFI_INVALID_PARAMETER;
   }
+
   mImageBase  = (UINT8 *) FileBuffer;
   mImageSize  = FileSize;
 
@@ -1303,7 +1339,8 @@ DxeImageVerificationHandler (
     //
     // It is not a valid Pe/Coff file.
     //
-    return EFI_ACCESS_DENIED;
+    Status = EFI_ACCESS_DENIED;
+    goto Done;
   }
 
   Magic = mNtHeader.Pe32->OptionalHeader.Magic;
@@ -1311,29 +1348,21 @@ DxeImageVerificationHandler (
     //
     // Use PE32 offset.
     //
+    NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
+    if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       mSecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
-  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+    }        
+  } else {
     //
     // Use PE32+ offset.
     //
+    NumberOfRvaAndSizes = mNtHeader.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes;
+    if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       mSecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
-  } else {
+    }
-    //
-    // Invalid header magic number.
-    //
-    Status       = EFI_INVALID_PARAMETER;
-    goto Done;
   }
 
-  if (mSecDataDir->VirtualAddress >= mImageSize) {
+  if ((mSecDataDir == NULL) || ((mSecDataDir != NULL) && (mSecDataDir->Size == 0))) {
-    //
-    // Sanity check to see if this file is corrupted.
-    //
-    Status       = EFI_INVALID_PARAMETER;
-    goto Done;
-  }
-
-  if (mSecDataDir->Size == 0) {
     //
     // This image is not signed.
     //
@@ -1474,8 +1503,6 @@ Done:
     FreePool (SignatureList);
   }
 
-  FreePool (SetupMode);
-
   return Status;
 }
 

SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c

@@ -256,7 +256,9 @@ TcgMeasureGptTable (
 
   @retval EFI_SUCCESS            Successfully measure image.
   @retval EFI_OUT_OF_RESOURCES   No enough resource to measure image.
+  @retval EFI_UNSUPPORTED        ImageType is unsupported or PE image is mal-format.  
   @retval other error value
+
 **/
 EFI_STATUS
 EFIAPI
@@ -282,14 +284,18 @@ TcgMeasurePeImage (
   UINTN                                HashSize;
   UINTN                                SumOfBytesHashed;
   EFI_IMAGE_SECTION_HEADER             *SectionHeader;
-  UINTN                             Index, Pos;
+  UINTN                                Index;
+  UINTN                                Pos;
   UINT16                               Magic;
   UINT32                               EventSize;
   UINT32                               EventNumber;
   EFI_PHYSICAL_ADDRESS                 EventLogLastEntry;
   EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr;
+  UINT32                               NumberOfRvaAndSizes;
+  BOOLEAN                              HashStatus;
+  UINT32                               CertSize;
 
-  Status        = EFI_SUCCESS;
+  Status        = EFI_UNSUPPORTED;
   ImageLoad     = NULL;
   SectionHeader = NULL;
   Sha1Ctx       = NULL;
@@ -326,7 +332,6 @@ TcgMeasurePeImage (
         "TcgMeasurePeImage: Unknown subsystem type %d",
         ImageType
         ));
-      Status = EFI_UNSUPPORTED;
       goto Finish;
   }
 
@@ -344,8 +349,9 @@ TcgMeasurePeImage (
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
     PeCoffHeaderOffset = DosHdr->e_lfanew;
   }
-  if (((EFI_TE_IMAGE_HEADER *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset))->Signature
+
-       == EFI_TE_IMAGE_HEADER_SIGNATURE) {
+  Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
+  if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
     goto Finish;
   }
 
@@ -367,13 +373,15 @@ TcgMeasurePeImage (
     goto Finish;
   }
 
-  Sha1Init (Sha1Ctx);
+  HashStatus = Sha1Init (Sha1Ctx);
+  if (!HashStatus) {
+    goto Finish;
+  }
 
   //
   // Measuring PE/COFF Image Header;
   // But CheckSum field and SECURITY data directory (certificate) are excluded
   //
-  Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
   Magic = Hdr.Pe32->OptionalHeader.Magic;
   
   //
@@ -385,19 +393,51 @@ TcgMeasurePeImage (
     //
     // Use PE32 offset
     //
+    NumberOfRvaAndSizes = Hdr.Pe32->OptionalHeader.NumberOfRvaAndSizes;
     HashSize = (UINTN) ((UINT8 *)(&Hdr.Pe32->OptionalHeader.CheckSum) - HashBase);
   } else {
     //
     // Use PE32+ offset
     //
+    NumberOfRvaAndSizes = Hdr.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes;
     HashSize = (UINTN) ((UINT8 *)(&Hdr.Pe32Plus->OptionalHeader.CheckSum) - HashBase);
   }
 
-  Sha1Update (Sha1Ctx, HashBase, HashSize);
+  HashStatus = Sha1Update (Sha1Ctx, HashBase, HashSize);
+  if (!HashStatus) {
+    goto Finish;
+  }  
 
   //
   // 5.  Skip over the image checksum (it occupies a single ULONG).
-  // 6.  Get the address of the beginning of the Cert Directory.
+  //
+  if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
+    //
+    // 6.  Since there is no Cert Directory in optional header, hash everything
+    //     from the end of the checksum to the end of image header.
+    //
+    if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+      //
+      // Use PE32 offset.
+      //
+      HashBase = (UINT8 *) &Hdr.Pe32->OptionalHeader.CheckSum + sizeof (UINT32);
+      HashSize = Hdr.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - ImageAddress);
+    } else {
+      //
+      // Use PE32+ offset.
+      //
+      HashBase = (UINT8 *) &Hdr.Pe32Plus->OptionalHeader.CheckSum + sizeof (UINT32);
+      HashSize = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - ImageAddress);
+    }
+
+    if (HashSize != 0) {
+      HashStatus  = Sha1Update (Sha1Ctx, HashBase, HashSize);
+      if (!HashStatus) {
+        goto Finish;
+      }
+    }    
+  } else {
+    //
     // 7.  Hash everything from the end of the checksum to the start of the Cert Directory.
     //
     if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
@@ -414,7 +454,12 @@ TcgMeasurePeImage (
       HashSize = (UINTN) ((UINT8 *)(&Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - HashBase);
     }
 
-  Sha1Update (Sha1Ctx, HashBase, HashSize);
+    if (HashSize != 0) {
+      HashStatus  = Sha1Update (Sha1Ctx, HashBase, HashSize);
+      if (!HashStatus) {
+        goto Finish;
+      }
+    }
 
     //
     // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
@@ -425,18 +470,22 @@ TcgMeasurePeImage (
       // Use PE32 offset
       //
       HashBase = (UINT8 *) &Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = Hdr.Pe32->OptionalHeader.SizeOfHeaders -
+      HashSize = Hdr.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - ImageAddress);
-             (UINTN) ((UINT8 *)(&Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - (UINT8 *) (UINTN) ImageAddress);
     } else {
       //
       // Use PE32+ offset
       //
       HashBase = (UINT8 *) &Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders -
+      HashSize = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) (HashBase - ImageAddress);
-             (UINTN) ((UINT8 *)(&Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - (UINT8 *) (UINTN) ImageAddress);
     }
     
-  Sha1Update (Sha1Ctx, HashBase, HashSize);
+    if (HashSize != 0) {
+      HashStatus  = Sha1Update (Sha1Ctx, HashBase, HashSize);
+      if (!HashStatus) {
+        goto Finish;
+      }
+    }
+  }
 
   //
   // 10. Set the SUM_OF_BYTES_HASHED to the size of the header
@@ -503,7 +552,10 @@ TcgMeasurePeImage (
     HashBase = (UINT8 *) (UINTN) ImageAddress + Section->PointerToRawData;
     HashSize = (UINTN) Section->SizeOfRawData;
 
-    Sha1Update (Sha1Ctx, HashBase, HashSize);
+    HashStatus = Sha1Update (Sha1Ctx, HashBase, HashSize);
+    if (!HashStatus) {
+      goto Finish;
+    }
 
     SumOfBytesHashed += HashSize;
   }
@@ -516,37 +568,42 @@ TcgMeasurePeImage (
   //
   if (ImageSize > SumOfBytesHashed) {
     HashBase = (UINT8 *) (UINTN) ImageAddress + SumOfBytesHashed;
-    if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+
-      if (ImageSize - SumOfBytesHashed < Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size) {
+    if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
-        Status = EFI_INVALID_PARAMETER;
+      CertSize = 0;
-        goto Finish;
-      }
-      //
-      // Use PE32 offset
-      //
-      HashSize = (UINTN)(ImageSize -
-                 Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size -
-                 SumOfBytesHashed);
     } else {
-      if (ImageSize - SumOfBytesHashed < Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size) {
+      if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-        Status = EFI_INVALID_PARAMETER;
+        //
-        goto Finish;
+        // Use PE32 offset.
+        //
+        CertSize = Hdr.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
+      } else {
+        //
+        // Use PE32+ offset.
+        //
+        CertSize = Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
       }
-      //
-      // Use PE32+ offset
-      //
-      HashSize = (UINTN)(ImageSize -
-                 Hdr.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY].Size -
-                 SumOfBytesHashed);      
     }
 
-    Sha1Update (Sha1Ctx, HashBase, HashSize);
+    if (ImageSize > CertSize + SumOfBytesHashed) {
+      HashSize = (UINTN) (ImageSize - CertSize - SumOfBytesHashed);
+
+      HashStatus = Sha1Update (Sha1Ctx, HashBase, HashSize);
+      if (!HashStatus) {
+        goto Finish;
+      }
+    } else if (ImageSize < CertSize + SumOfBytesHashed) {
+      goto Finish;
+    }
   }
 
   //
   // 17.  Finalize the SHA hash.
   //
-  Sha1Final (Sha1Ctx, (UINT8 *)&TcgEvent->Digest);
+  HashStatus = Sha1Final (Sha1Ctx, (UINT8 *) &TcgEvent->Digest);
+  if (!HashStatus) {
+    goto Finish;
+  }
 
   //
   // Log the PE data