[edk2-devel] [PATCH 4/9] SecurityPkg/DxeImageVerificationLib: avoid bypass in fetching dbx in IsAllowedByDb(CVE-2019-14575)

[Yao, Jiewen]

Reviewed-by: Jiewen Yao <Jiewen.yao at intel.com>

> -----Original Message-----
> From: Wang, Jian J <jian.j.wang at intel.com>
> Sent: Thursday, February 6, 2020 10:19 PM
> To: devel at edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao at intel.com>; Zhang, Chao B
> <chao.b.zhang at 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 at intel.com>
> Cc: Chao Zhang <chao.b.zhang at intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-