SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc fail

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>
Message-Id: <20200116190705.18816-11-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
 Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
 <d3fbb76dabed4e1987c512c328c82810@intel.com>]

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index c98b9e45923f48063198c8856c73d5eae423734f..015a5b61a3012860f0a9452b074e02659f0cb451 100644 (file)
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -704,7 +704,7 @@ GetImageExeInfoTableSize (
   @param[in]  Name            Input a null-terminated, user-friendly name.\r
   @param[in]  DevicePath      Input device path pointer.\r
   @param[in]  Signature       Input signature info in EFI_SIGNATURE_LIST data structure.\r
-  @param[in]  SignatureSize   Size of signature.\r
+  @param[in]  SignatureSize   Size of signature. Must be zero if Signature is NULL.\r
 \r
 **/\r
 VOID\r
@@ -761,6 +761,7 @@ AddImageExeInfo (
   //\r
   // Signature size can be odd. Pad after signature to ensure next EXECUTION_INFO entry align\r
   //\r
+  ASSERT (Signature != NULL || SignatureSize == 0);\r
   NewImageExeInfoEntrySize = sizeof (EFI_IMAGE_EXECUTION_INFO) + NameStringLen + DevicePathSize + SignatureSize;\r
 \r
   NewImageExeInfoTable      = (EFI_IMAGE_EXECUTION_INFO_TABLE *) AllocateRuntimePool (ImageExeInfoTableSize + NewImageExeInfoEntrySize);\r
@@ -1858,6 +1859,7 @@ DxeImageVerificationHandler (
     SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize;\r
     SignatureList     = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize);\r
     if (SignatureList == NULL) {\r
+      SignatureListSize = 0;\r
       goto Failed;\r
     }\r
     SignatureList->SignatureHeaderSize  = 0;\r