summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGerd Hoffmann <kraxel@redhat.com>2023-03-03 18:35:53 +0800
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2023-03-21 05:52:23 +0000
commit494127613b36e870250649b02cd4ce5f1969d9bd (patch)
tree21876f8b9d196c95fc5d880595442df3f132a557
parentb7a8264ae4704f781e70cc44dafdf07e4e5e690a (diff)
downloadedk2-494127613b36e870250649b02cd4ce5f1969d9bd.tar.gz
edk2-494127613b36e870250649b02cd4ce5f1969d9bd.tar.bz2
edk2-494127613b36e870250649b02cd4ce5f1969d9bd.zip
SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2
Call gRT->GetVariable() directly to read the SecureBoot variable. It is one byte in size so we can easily place it on the stack instead of having GetEfiGlobalVariable2() allocate it for us, which avoids a few possible error cases. Skip secure boot checks if (and only if): (a) the SecureBoot variable is not present (EFI_NOT_FOUND) according to the return value, or (b) the SecureBoot variable was read successfully and is set to SECURE_BOOT_MODE_DISABLE. Previously the code skipped the secure boot checks on *any* gRT->GetVariable() error (GetEfiGlobalVariable2 sets the variable value to NULL in that case) and also on memory allocation failures. Fixes: CVE-2019-14560 Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2167 Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Suggested-by: Marvin Häuser <mhaeuser@posteo.de> Reviewed-by: Min Xu <min.m.xu@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
-rw-r--r--SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c19
1 files changed, 12 insertions, 7 deletions
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 66e2f5eaa3..b3d40c21e9 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1671,7 +1671,8 @@ DxeImageVerificationHandler (
EFI_IMAGE_EXECUTION_ACTION Action;
WIN_CERTIFICATE *WinCertificate;
UINT32 Policy;
- UINT8 *SecureBoot;
+ UINT8 SecureBoot;
+ UINTN SecureBootSize;
PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
UINT32 NumberOfRvaAndSizes;
WIN_CERTIFICATE_EFI_PKCS *PkcsCertData;
@@ -1686,6 +1687,8 @@ DxeImageVerificationHandler (
RETURN_STATUS PeCoffStatus;
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
+ EFI_STATUS VarStatus;
+ UINT32 VarAttr;
BOOLEAN IsFound;
SignatureList = NULL;
@@ -1742,24 +1745,26 @@ DxeImageVerificationHandler (
CpuDeadLoop ();
}
- GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID **)&SecureBoot, NULL);
+ SecureBootSize = sizeof (SecureBoot);
+ VarStatus = gRT->GetVariable (EFI_SECURE_BOOT_MODE_NAME, &gEfiGlobalVariableGuid, &VarAttr, &SecureBootSize, &SecureBoot);
//
// Skip verification if SecureBoot variable doesn't exist.
//
- if (SecureBoot == NULL) {
+ if (VarStatus == EFI_NOT_FOUND) {
return EFI_SUCCESS;
}
//
// Skip verification if SecureBoot is disabled but not AuditMode
//
- if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) {
- FreePool (SecureBoot);
+ if ((VarStatus == EFI_SUCCESS) &&
+ (VarAttr == (EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS)) &&
+ (SecureBoot == SECURE_BOOT_MODE_DISABLE))
+ {
return EFI_SUCCESS;
}
- FreePool (SecureBoot);
-
//
// Read the Dos header.
//