From: "Gerd Hoffmann" <kraxel@redhat.com>
To: devel@edk2.groups.io
Cc: "Pawel Polawski" <ppolawsk@redhat.com>,
"Jiewen Yao" <jiewen.yao@intel.com>,
"Marvin Häuser" <mhaeuser@posteo.de>,
jmaloy@redhat.com, "Min Xu" <min.m.xu@intel.com>,
"Jian J Wang" <jian.j.wang@intel.com>,
"Oliver Steffen" <osteffen@redhat.com>,
"Gerd Hoffmann" <kraxel@redhat.com>
Subject: [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2
Date: Mon, 27 Feb 2023 13:55:17 +0100 [thread overview]
Message-ID: <20230227125517.821136-1-kraxel@redhat.com> (raw)
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>
---
.../DxeImageVerificationLib.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 66e2f5eaa3c0..f29a27e5a053 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,7 @@ DxeImageVerificationHandler (
RETURN_STATUS PeCoffStatus;
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
+ EFI_STATUS VarStatus;
BOOLEAN IsFound;
SignatureList = NULL;
@@ -1742,24 +1744,22 @@ DxeImageVerificationHandler (
CpuDeadLoop ();
}
- GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID **)&SecureBoot, NULL);
+ SecureBootSize = sizeof (SecureBoot);
+ VarStatus = gRT->GetVariable (EFI_SECURE_BOOT_MODE_NAME, &gEfiGlobalVariableGuid, NULL, &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) && (SecureBoot == SECURE_BOOT_MODE_DISABLE)) {
return EFI_SUCCESS;
}
- FreePool (SecureBoot);
-
//
// Read the Dos header.
//
--
2.39.2
next reply other threads:[~2023-02-27 12:55 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-27 12:55 Gerd Hoffmann [this message]
2023-02-27 15:53 ` [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2 Marvin Häuser
2023-02-28 6:12 ` Gerd Hoffmann
2023-02-28 8:47 ` Marvin Häuser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230227125517.821136-1-kraxel@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox