You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
110 lines
4.2 KiB
110 lines
4.2 KiB
9 months ago
|
From bb0f29580825e60a5dc5c67e260dd20258eb71b0 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Jon Maloy <jmaloy@redhat.com>
|
||
|
|
Date: Wed, 29 Mar 2023 11:52:52 -0400
|
||
|
|
Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Check result of
|
||
|
|
GetEfiGlobalVariable2
|
||
|
|
MIME-Version: 1.0
|
||
|
|
Content-Type: text/plain; charset=UTF-8
|
||
|
|
Content-Transfer-Encoding: 8bit
|
||
|
|
|
||
|
|
RH-Author: Jon Maloy <jmaloy@redhat.com>
|
||
|
|
RH-MergeRequest: 22: SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2
|
||
|
|
RH-Bugzilla: 1861743
|
||
|
|
RH-Acked-by: Gerd Hoffmann <kraxel@redhat.com>
|
||
|
|
RH-Commit: [1/1] 70e1ae5e2c7c148fc23160acdd360c044df5f4ff
|
||
|
|
|
||
|
|
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1861743
|
||
|
|
Upstream: Merged
|
||
|
|
CVE: CVE-2019-14560
|
||
|
|
|
||
|
|
commit 494127613b36e870250649b02cd4ce5f1969d9bd
|
||
|
|
Author: Gerd Hoffmann <kraxel@redhat.com>
|
||
|
|
Date: Fri Mar 3 18:35:53 2023 +0800
|
||
|
|
|
||
|
|
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
|
||
|
|
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>
|
||
|
|
|
||
|
|
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
|
||
|
|
---
|
||
|
|
.../DxeImageVerificationLib.c | 18 ++++++++++++------
|
||
|
|
1 file changed, 12 insertions(+), 6 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||
|
|
index c48861cd64..1252927664 100644
|
||
|
|
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||
|
|
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||
|
|
@@ -1650,7 +1650,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;
|
||
|
|
@@ -1665,6 +1666,8 @@ DxeImageVerificationHandler (
|
||
|
|
RETURN_STATUS PeCoffStatus;
|
||
|
|
EFI_STATUS HashStatus;
|
||
|
|
EFI_STATUS DbStatus;
|
||
|
|
+ EFI_STATUS VarStatus;
|
||
|
|
+ UINT32 VarAttr;
|
||
|
|
BOOLEAN IsFound;
|
||
|
|
|
||
|
|
SignatureList = NULL;
|
||
|
|
@@ -1720,22 +1723,25 @@ 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.
|
||
|
|
--
|
||
|
|
2.39.1
|
||
|
|
|