public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wang, Jian J" <jian.j.wang@intel.com>
To: devel@edk2.groups.io
Cc: Jiewen Yao <jiewen.yao@intel.com>, Chao Zhang <chao.b.zhang@intel.com>
Subject: [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx in IsAllowedByDb(CVE-2019-14575)
Date: Thu,  6 Feb 2020 22:19:28 +0800	[thread overview]
Message-ID: <20200206141933.356-5-jian.j.wang@intel.com> (raw)
In-Reply-To: <20200206141933.356-1-jian.j.wang@intel.com>

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>
---
 .../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


  parent reply	other threads:[~2020-02-06 14:19 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 14:19 [PATCH 0/9] Fix false negative issue in DxeImageVerificationHandler Wang, Jian J
2020-02-06 14:19 ` [PATCH 1/9] SecurityPkg/DxeImageVerificationLib: Fix memory leaks(CVE-2019-14575) Wang, Jian J
2020-02-13  9:34   ` Yao, Jiewen
2020-02-13 16:43   ` [edk2-devel] " Philippe Mathieu-Daudé
2020-02-06 14:19 ` [PATCH 2/9] SecurityPkg/DxeImageVerificationLib: reject CertStack.CertNumber==0 per DBX(CVE-2019-14575) Wang, Jian J
2020-02-13  9:36   ` Yao, Jiewen
2020-02-06 14:19 ` [PATCH 3/9] SecurityPkg/DxeImageVerificationLib: fix wrong fetching dbx in IsAllowedByDb(CVE-2019-14575) Wang, Jian J
2020-02-13  9:38   ` Yao, Jiewen
2020-02-06 14:19 ` Wang, Jian J [this message]
2020-02-13  9:39   ` [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in " Yao, Jiewen
2020-02-06 14:19 ` [PATCH 5/9] SecurityPkg/DxeImageVerificationLib: refactor db/dbx fetching code " Wang, Jian J
2020-02-13  9:44   ` Yao, Jiewen
2020-02-06 14:19 ` [PATCH 6/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsCertHashFoundInDatabase(CVE-2019-14575) Wang, Jian J
2020-02-13 10:11   ` Yao, Jiewen
2020-02-13 15:07     ` Wang, Jian J
2020-02-14  0:54       ` Yao, Jiewen
2020-02-14  3:31         ` Wang, Jian J
2020-02-14  3:33           ` Yao, Jiewen
2020-02-06 14:19 ` [PATCH 7/9] SecurityPkg/DxeImageVerificationLib: tighten default result of IsForbiddenByDbx()(CVE-2019-14575) Wang, Jian J
2020-02-13 10:13   ` Yao, Jiewen
2020-02-06 14:19 ` [PATCH 8/9] SecurityPkg/DxeImageVerificationLib: plug Data leak in IsForbiddenByDbx()(CVE-2019-14575) Wang, Jian J
2020-02-13 10:14   ` Yao, Jiewen
2020-02-13 16:56   ` [edk2-devel] " Philippe Mathieu-Daudé
2020-02-06 14:19 ` [PATCH 9/9] SecurityPkg/DxeImageVerificationLib: Differentiate error and search result in IsSignatureFoundInDatabase(CVE-2019-14575) Wang, Jian J
2020-02-13  9:02   ` [edk2-devel] " Zhang, Chao B
2020-02-13 10:20   ` Yao, Jiewen
2020-02-13  1:53 ` [edk2-devel] [PATCH 0/9] Fix false negative issue in DxeImageVerificationHandler Liming Gao

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=20200206141933.356-5-jian.j.wang@intel.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