From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com []) by mx.groups.io with SMTP id smtpd.web11.3343.1581665266656025367 for ; Thu, 13 Feb 2020 23:27:48 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=fail (domain: intel.com, ip: , mailfrom: jian.j.wang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2020 23:27:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,439,1574150400"; d="scan'208";a="347904209" Received: from shwdeopensfp777.ccr.corp.intel.com ([10.239.158.78]) by fmsmga001.fm.intel.com with ESMTP; 13 Feb 2020 23:27:48 -0800 From: "Wang, Jian J" To: devel@edk2.groups.io Cc: Jiewen Yao , Chao Zhang Subject: [PATCH v2 03/10] SecurityPkg/DxeImageVerificationLib: fix wrong fetch dbx in IsAllowedByDb(CVE-2019-14575) Date: Fri, 14 Feb 2020 15:27:38 +0800 Message-Id: <20200214072745.1570-4-jian.j.wang@intel.com> X-Mailer: git-send-email 2.24.0.windows.2 In-Reply-To: <20200214072745.1570-1-jian.j.wang@intel.com> References: <20200214072745.1570-1-jian.j.wang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1608 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 =3D=3D EFI_BUFFER_TOO_SMALL) { goto Done; } Cc: Jiewen Yao Cc: Chao Zhang Signed-off-by: Jian J Wang Reviewed-by: Jiewen Yao --- .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 5dcd6efed5..1efb2f96cd 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1456,8 +1456,9 @@ IsAllowedByDb ( //=0D // Here We still need to check if this RootCert's Hash is revo= ked=0D //=0D + DbxDataSize =3D 0;=0D Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &= gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);=0D - if (Status =3D=3D EFI_BUFFER_TOO_SMALL) {=0D + if (Status !=3D EFI_BUFFER_TOO_SMALL) {=0D goto Done;=0D }=0D DbxData =3D (UINT8 *) AllocateZeroPool (DbxDataSize);=0D --=20 2.24.0.windows.2