Skip to content

Commit

Permalink
SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx (CV…
Browse files Browse the repository at this point in the history
…E-2019-14575)

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@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
  • Loading branch information
Jian J Wang authored and mergify[bot] committed Feb 19, 2020
1 parent 9e56970 commit 929d1a2
Showing 1 changed file with 11 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 929d1a2

Please sign in to comment.