Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Thursday, February 6, 2020 10:19 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in
> fetching dbx in IsAllowedByDb(CVE-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>
> ---
> .../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 (#54348): https://edk2.groups.io/g/devel/message/54348
Mute This Topic: https://groups.io/mt/71023421/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-