[PATCH 10/11] SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc fail

["Laszlo Ersek" <lersek@redhat.com>]

It makes no sense to call AddImageExeInfo() with (Signature == NULL) and
(SignatureSize > 0). AddImageExeInfo() does not crash in such a case -- it
avoids the CopyMem() call --, but it creates an invalid
EFI_IMAGE_EXECUTION_INFO record. Namely, the
"EFI_IMAGE_EXECUTION_INFO.InfoSize" field includes "SignatureSize", but
the actual signature bytes are not filled in.

Document and ASSERT() this condition in AddImageExeInfo().

In DxeImageVerificationHandler(), zero out "SignatureListSize" if we set
"SignatureList" to NULL due to AllocateZeroPool() failure.

(Another approach could be to avoid calling AddImageExeInfo() completely,
in case AllocateZeroPool() fails. Unfortunately, the UEFI v2.8 spec does
not seem to state clearly whether a signature is mandatory in
EFI_IMAGE_EXECUTION_INFO, if the "Action" field is
EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED or EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND.

For now, the EFI_IMAGE_EXECUTION_INFO addition logic is not changed; we
only make sure that the record we add is not malformed.)

Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index f6ad5931be07..53ef340c08ad 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -678,151 +678,152 @@ UINTN
 GetImageExeInfoTableSize (
   EFI_IMAGE_EXECUTION_INFO_TABLE        *ImageExeInfoTable
   )
 {
   UINTN                     Index;
   EFI_IMAGE_EXECUTION_INFO  *ImageExeInfoItem;
   UINTN                     TotalSize;
 
   if (ImageExeInfoTable == NULL) {
     return 0;
   }
 
   ImageExeInfoItem  = (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) ImageExeInfoTable + sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE));
   TotalSize         = sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE);
   for (Index = 0; Index < ImageExeInfoTable->NumberOfImages; Index++) {
     TotalSize += ReadUnaligned32 ((UINT32 *) &ImageExeInfoItem->InfoSize);
     ImageExeInfoItem = (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) ImageExeInfoItem + ReadUnaligned32 ((UINT32 *) &ImageExeInfoItem->InfoSize));
   }
 
   return TotalSize;
 }
 
 /**
   Create an Image Execution Information Table entry and add it to system configuration table.
 
   @param[in]  Action          Describes the action taken by the firmware regarding this image.
   @param[in]  Name            Input a null-terminated, user-friendly name.
   @param[in]  DevicePath      Input device path pointer.
   @param[in]  Signature       Input signature info in EFI_SIGNATURE_LIST data structure.
-  @param[in]  SignatureSize   Size of signature.
+  @param[in]  SignatureSize   Size of signature. Must be zero if Signature is NULL.
 
 **/
 VOID
 AddImageExeInfo (
   IN       EFI_IMAGE_EXECUTION_ACTION       Action,
   IN       CHAR16                           *Name OPTIONAL,
   IN CONST EFI_DEVICE_PATH_PROTOCOL         *DevicePath,
   IN       EFI_SIGNATURE_LIST               *Signature OPTIONAL,
   IN       UINTN                            SignatureSize
   )
 {
   EFI_IMAGE_EXECUTION_INFO_TABLE  *ImageExeInfoTable;
   EFI_IMAGE_EXECUTION_INFO_TABLE  *NewImageExeInfoTable;
   EFI_IMAGE_EXECUTION_INFO        *ImageExeInfoEntry;
   UINTN                           ImageExeInfoTableSize;
   UINTN                           NewImageExeInfoEntrySize;
   UINTN                           NameStringLen;
   UINTN                           DevicePathSize;
   CHAR16                          *NameStr;
 
   ImageExeInfoTable     = NULL;
   NewImageExeInfoTable  = NULL;
   ImageExeInfoEntry     = NULL;
   NameStringLen         = 0;
   NameStr               = NULL;
 
   if (DevicePath == NULL) {
     return ;
   }
 
   if (Name != NULL) {
     NameStringLen = StrSize (Name);
   } else {
     NameStringLen = sizeof (CHAR16);
   }
 
   EfiGetSystemConfigurationTable (&gEfiImageSecurityDatabaseGuid, (VOID **) &ImageExeInfoTable);
   if (ImageExeInfoTable != NULL) {
     //
     // The table has been found!
     // We must enlarge the table to accommodate the new exe info entry.
     //
     ImageExeInfoTableSize = GetImageExeInfoTableSize (ImageExeInfoTable);
   } else {
     //
     // Not Found!
     // We should create a new table to append to the configuration table.
     //
     ImageExeInfoTableSize = sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE);
   }
 
   DevicePathSize            = GetDevicePathSize (DevicePath);
 
   //
   // Signature size can be odd. Pad after signature to ensure next EXECUTION_INFO entry align
   //
+  ASSERT (Signature != NULL || SignatureSize == 0);
   NewImageExeInfoEntrySize = sizeof (EFI_IMAGE_EXECUTION_INFO) + NameStringLen + DevicePathSize + SignatureSize;
 
   NewImageExeInfoTable      = (EFI_IMAGE_EXECUTION_INFO_TABLE *) AllocateRuntimePool (ImageExeInfoTableSize + NewImageExeInfoEntrySize);
   if (NewImageExeInfoTable == NULL) {
     return ;
   }
 
   if (ImageExeInfoTable != NULL) {
     CopyMem (NewImageExeInfoTable, ImageExeInfoTable, ImageExeInfoTableSize);
   } else {
     NewImageExeInfoTable->NumberOfImages = 0;
   }
   NewImageExeInfoTable->NumberOfImages++;
   ImageExeInfoEntry = (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) NewImageExeInfoTable + ImageExeInfoTableSize);
   //
   // Update new item's information.
   //
   WriteUnaligned32 ((UINT32 *) ImageExeInfoEntry, Action);
   WriteUnaligned32 ((UINT32 *) ((UINT8 *) ImageExeInfoEntry + sizeof (EFI_IMAGE_EXECUTION_ACTION)), (UINT32) NewImageExeInfoEntrySize);
 
   NameStr = (CHAR16 *)(ImageExeInfoEntry + 1);
   if (Name != NULL) {
     CopyMem ((UINT8 *) NameStr, Name, NameStringLen);
   } else {
     ZeroMem ((UINT8 *) NameStr, sizeof (CHAR16));
   }
 
   CopyMem (
     (UINT8 *) NameStr + NameStringLen,
     DevicePath,
     DevicePathSize
     );
   if (Signature != NULL) {
     CopyMem (
       (UINT8 *) NameStr + NameStringLen + DevicePathSize,
       Signature,
       SignatureSize
       );
   }
   //
   // Update/replace the image execution table.
   //
   gBS->InstallConfigurationTable (&gEfiImageSecurityDatabaseGuid, (VOID *) NewImageExeInfoTable);
 
   //
   // Free Old table data!
   //
   if (ImageExeInfoTable != NULL) {
     FreePool (ImageExeInfoTable);
   }
 }
 
 /**
   Check whether the hash of an given X.509 certificate is in forbidden database (DBX).
 
   @param[in]  Certificate       Pointer to X.509 Certificate that is searched for.
   @param[in]  CertSize          Size of X.509 Certificate.
   @param[in]  SignatureList     Pointer to the Signature List in forbidden database.
   @param[in]  SignatureListSize Size of Signature List.
   @param[out] RevocationTime    Return the time that the certificate was revoked.
 
   @return TRUE   The certificate hash is found in the forbidden database.
   @return FALSE  The certificate hash is not found in the forbidden database.
 
 **/
@@ -1555,343 +1556,344 @@ EFIAPI
 DxeImageVerificationHandler (
   IN  UINT32                           AuthenticationStatus,
   IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File,
   IN  VOID                             *FileBuffer,
   IN  UINTN                            FileSize,
   IN  BOOLEAN                          BootPolicy
   )
 {
   EFI_IMAGE_DOS_HEADER                 *DosHdr;
   BOOLEAN                              IsVerified;
   EFI_SIGNATURE_LIST                   *SignatureList;
   UINTN                                SignatureListSize;
   EFI_SIGNATURE_DATA                   *Signature;
   EFI_IMAGE_EXECUTION_ACTION           Action;
   WIN_CERTIFICATE                      *WinCertificate;
   UINT32                               Policy;
   UINT8                                *SecureBoot;
   PE_COFF_LOADER_IMAGE_CONTEXT         ImageContext;
   UINT32                               NumberOfRvaAndSizes;
   WIN_CERTIFICATE_EFI_PKCS             *PkcsCertData;
   WIN_CERTIFICATE_UEFI_GUID            *WinCertUefiGuid;
   UINT8                                *AuthData;
   UINTN                                AuthDataSize;
   EFI_IMAGE_DATA_DIRECTORY             *SecDataDir;
   UINT32                               OffSet;
   CHAR16                               *NameStr;
   RETURN_STATUS                        PeCoffStatus;
   EFI_STATUS                           HashStatus;
 
   SignatureList     = NULL;
   SignatureListSize = 0;
   WinCertificate    = NULL;
   SecDataDir        = NULL;
   PkcsCertData      = NULL;
   Action            = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
   IsVerified        = FALSE;
 
 
   //
   // Check the image type and get policy setting.
   //
   switch (GetImageType (File)) {
 
   case IMAGE_FROM_FV:
     Policy = ALWAYS_EXECUTE;
     break;
 
   case IMAGE_FROM_OPTION_ROM:
     Policy = PcdGet32 (PcdOptionRomImageVerificationPolicy);
     break;
 
   case IMAGE_FROM_REMOVABLE_MEDIA:
     Policy = PcdGet32 (PcdRemovableMediaImageVerificationPolicy);
     break;
 
   case IMAGE_FROM_FIXED_MEDIA:
     Policy = PcdGet32 (PcdFixedMediaImageVerificationPolicy);
     break;
 
   default:
     Policy = DENY_EXECUTE_ON_SECURITY_VIOLATION;
     break;
   }
   //
   // If policy is always/never execute, return directly.
   //
   if (Policy == ALWAYS_EXECUTE) {
     return EFI_SUCCESS;
   }
   if (Policy == NEVER_EXECUTE) {
     return EFI_ACCESS_DENIED;
   }
 
   //
   // The policy QUERY_USER_ON_SECURITY_VIOLATION and ALLOW_EXECUTE_ON_SECURITY_VIOLATION
   // violates the UEFI spec and has been removed.
   //
   ASSERT (Policy != QUERY_USER_ON_SECURITY_VIOLATION && Policy != ALLOW_EXECUTE_ON_SECURITY_VIOLATION);
   if (Policy == QUERY_USER_ON_SECURITY_VIOLATION || Policy == ALLOW_EXECUTE_ON_SECURITY_VIOLATION) {
     CpuDeadLoop ();
   }
 
   GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID**)&SecureBoot, NULL);
   //
   // Skip verification if SecureBoot variable doesn't exist.
   //
   if (SecureBoot == NULL) {
     return EFI_SUCCESS;
   }
 
   //
   // Skip verification if SecureBoot is disabled but not AuditMode
   //
   if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) {
     FreePool (SecureBoot);
     return EFI_SUCCESS;
   }
   FreePool (SecureBoot);
 
   //
   // Read the Dos header.
   //
   if (FileBuffer == NULL) {
     return EFI_ACCESS_DENIED;
   }
 
   mImageBase  = (UINT8 *) FileBuffer;
   mImageSize  = FileSize;
 
   ZeroMem (&ImageContext, sizeof (ImageContext));
   ImageContext.Handle    = (VOID *) FileBuffer;
   ImageContext.ImageRead = (PE_COFF_LOADER_READ_FILE) DxeImageVerificationLibImageRead;
 
   //
   // Get information about the image being loaded
   //
   PeCoffStatus = PeCoffLoaderGetImageInfo (&ImageContext);
   if (RETURN_ERROR (PeCoffStatus)) {
     //
     // The information can't be got from the invalid PeImage
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: PeImage invalid. Cannot retrieve image information.\n"));
     goto Failed;
   }
 
   DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase;
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
     //
     // DOS image header is present,
     // so read the PE header after the DOS image header.
     //
     mPeCoffHeaderOffset = DosHdr->e_lfanew;
   } else {
     mPeCoffHeaderOffset = 0;
   }
   //
   // Check PE/COFF image.
   //
   mNtHeader.Pe32 = (EFI_IMAGE_NT_HEADERS32 *) (mImageBase + mPeCoffHeaderOffset);
   if (mNtHeader.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
     //
     // It is not a valid Pe/Coff file.
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Not a valid PE/COFF image.\n"));
     goto Failed;
   }
 
   if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
     //
     // Use PE32 offset.
     //
     NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
     if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       SecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
     }
   } else {
     //
     // Use PE32+ offset.
     //
     NumberOfRvaAndSizes = mNtHeader.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes;
     if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       SecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
     }
   }
 
   //
   // Start Image Validation.
   //
   if (SecDataDir == NULL || SecDataDir->Size == 0) {
     //
     // This image is not signed. The SHA256 hash value of the image must match a record in the security database "db",
     // and not be reflected in the security data base "dbx".
     //
     if (!HashPeImage (HASHALG_SHA256)) {
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image using %s.\n", mHashTypeStr));
       goto Failed;
     }
 
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) {
       //
       // Image Hash is in forbidden database (DBX).
       //
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
       goto Failed;
     }
 
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) {
       //
       // Image Hash is in allowed database (DB).
       //
       return EFI_SUCCESS;
     }
 
     //
     // Image Hash is not found in both forbidden and allowed database.
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
     goto Failed;
   }
 
   //
   // Verify the signature of the image, multiple signatures are allowed as per PE/COFF Section 4.7
   // "Attribute Certificate Table".
   // The first certificate starts at offset (SecDataDir->VirtualAddress) from the start of the file.
   //
   for (OffSet = SecDataDir->VirtualAddress;
        OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
        OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))) {
     WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
     if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof (WIN_CERTIFICATE) ||
         (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate->dwLength) {
       break;
     }
 
     //
     // Verify the image's Authenticode signature, only DER-encoded PKCS#7 signed data is supported.
     //
     if (WinCertificate->wCertificateType == WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
       //
       // The certificate is formatted as WIN_CERTIFICATE_EFI_PKCS which is described in the
       // Authenticode specification.
       //
       PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *) WinCertificate;
       if (PkcsCertData->Hdr.dwLength <= sizeof (PkcsCertData->Hdr)) {
         break;
       }
       AuthData   = PkcsCertData->CertData;
       AuthDataSize = PkcsCertData->Hdr.dwLength - sizeof(PkcsCertData->Hdr);
     } else if (WinCertificate->wCertificateType == WIN_CERT_TYPE_EFI_GUID) {
       //
       // The certificate is formatted as WIN_CERTIFICATE_UEFI_GUID which is described in UEFI Spec.
       //
       WinCertUefiGuid = (WIN_CERTIFICATE_UEFI_GUID *) WinCertificate;
       if (WinCertUefiGuid->Hdr.dwLength <= OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
         break;
       }
       if (!CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid)) {
         continue;
       }
       AuthData = WinCertUefiGuid->CertData;
       AuthDataSize = WinCertUefiGuid->Hdr.dwLength - OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
     } else {
       if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) {
         break;
       }
       continue;
     }
 
     HashStatus = HashPeImageByType (AuthData, AuthDataSize);
     if (EFI_ERROR (HashStatus)) {
       continue;
     }
 
     //
     // Check the digital signature against the revoked certificate in forbidden database (dbx).
     //
     if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
       Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
       IsVerified = FALSE;
       break;
     }
 
     //
     // Check the digital signature against the valid certificate in allowed database (db).
     //
     if (!IsVerified) {
       if (IsAllowedByDb (AuthData, AuthDataSize)) {
         IsVerified = TRUE;
       }
     }
 
     //
     // Check the image's hash value.
     //
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) {
       Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr));
       IsVerified = FALSE;
       break;
     }
     if (!IsVerified) {
       if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) {
         IsVerified = TRUE;
       } else {
         DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
       }
     }
   }
 
   if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
     //
     // The Size in Certificate Table or the attribute certificate table is corrupted.
     //
     IsVerified = FALSE;
   }
 
   if (IsVerified) {
     return EFI_SUCCESS;
   }
   if (Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) {
     //
     // Get image hash value as signature of executable.
     //
     SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize;
     SignatureList     = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize);
     if (SignatureList == NULL) {
+      SignatureListSize = 0;
       goto Failed;
     }
     SignatureList->SignatureHeaderSize  = 0;
     SignatureList->SignatureListSize    = (UINT32) SignatureListSize;
     SignatureList->SignatureSize        = (UINT32) (sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize);
     CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID));
     Signature = (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + sizeof (EFI_SIGNATURE_LIST));
     CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize);
   }
 
 Failed:
   //
   // Policy decides to defer or reject the image; add its information in image executable information table.
   //
   NameStr = ConvertDevicePathToText (File, FALSE, TRUE);
   AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize);
   if (NameStr != NULL) {
     DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n", NameStr));
     FreePool(NameStr);
   }
 
   if (SignatureList != NULL) {
     FreePool (SignatureList);
   }
 
   return EFI_SECURITY_VIOLATION;
 }
 
 /**
   On Ready To Boot Services Event notification handler.
 
   Add the image execution information table if it is not in system configuration table.
 
   @param[in]  Event     Event whose notification function is being invoked
   @param[in]  Context   Pointer to the notification function's context
 
 **/
-- 
2.19.1.3.g30247aa5d201