[edk2-devel] [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx in IsAllowedByDb(CVE-2019-14575)

[Wang, Jian J]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608

In timestamp check after the cert is found in db, the original code jumps
to 'Done' if any error happens in fetching dbx variable. At any of the jump,
VerifyStatus equals to TRUE, which means allowed-by-db. This should not
be allowed except to EFI_NOT_FOUND case (meaning dbx doesn't exist), because
it could be used to bypass timestamp check.

This patch add code to change VerifyStatus to FALSE in the case of memory
allocation failure and dbx fetching failure to avoid potential bypass issue.

Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Chao Zhang <chao.b.zhang at intel.com>
Signed-off-by: Jian J Wang <jian.j.wang at intel.com>
---
 .../DxeImageVerificationLib/DxeImageVerificationLib.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 1efb2f96cd..ed5dbf26b0 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1459,15 +1459,26 @@ IsAllowedByDb (
             DbxDataSize = 0;
             Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);
             if (Status != EFI_BUFFER_TOO_SMALL) {
+              if (Status != EFI_NOT_FOUND) {
+                VerifyStatus = FALSE;
+              }
               goto Done;
             }
             DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize);
             if (DbxData == NULL) {
+              //
+              // Force not-allowed-by-db to avoid bypass
+              //
+              VerifyStatus = FALSE;
               goto Done;
             }
 
             Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData);
             if (EFI_ERROR (Status)) {
+              //
+              // Force not-allowed-by-db to avoid bypass
+              //
+              VerifyStatus = FALSE;
               goto Done;
             }
 
-- 
2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53870): https://edk2.groups.io/g/devel/message/53870
Mute This Topic: https://groups.io/mt/71023421/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-