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 3/9] SecurityPkg/DxeImageVerificationLib: fix wrong fetching dbx in IsAllowedByDb(CVE-2019-14575)
Date: Thu,  6 Feb 2020 22:19:27 +0800	[thread overview]
Message-ID: <20200206141933.356-4-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

Normally two times of calling gRT->GetVariable() are needed to get
the data of a variable: get the variable size by passing zero variable
size, and then allocate enough memory and pass the correct variable size
and buffer.

But in the inner loop in IsAllowedByDb(), the DbxDataSize was not
initialized to zero before calling gRT->GetVariable(). It won't cause
problem if dbx does not exist. But it will give wrong result if dbx
exists and the DbxDataSize happens to be a small enough value. In this
situation, EFI_BUFFER_TOO_SMALL will be returned. Then the result check
code followed will jump to 'Done', which is not correct because it's
actually the value expected.

            if (Status == EFI_BUFFER_TOO_SMALL) {
              goto Done;
            }

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>
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c  | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 5dcd6efed5..1efb2f96cd 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1456,8 +1456,9 @@ IsAllowedByDb (
             //
             // Here We still need to check if this RootCert's Hash is revoked
             //
+            DbxDataSize = 0;
             Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);
-            if (Status == EFI_BUFFER_TOO_SMALL) {
+            if (Status != EFI_BUFFER_TOO_SMALL) {
               goto Done;
             }
             DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize);
-- 
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 ` Wang, Jian J [this message]
2020-02-13  9:38   ` [PATCH 3/9] SecurityPkg/DxeImageVerificationLib: fix wrong fetching dbx in IsAllowedByDb(CVE-2019-14575) Yao, Jiewen
2020-02-06 14:19 ` [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in " Wang, Jian J
2020-02-13  9:39   ` 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-4-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