Re: [edk2-devel] [PATCH 1/9] SecurityPkg/DxeImageVerificationLib: Fix memory leaks(CVE-2019-14575)

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 2/6/20 3:19 PM, Wang, Jian J wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
> 
> Pointer HashCtx used in IsCertHashFoundInDatabase() is not freed inside
> the while-loop, if it will run more than once.

By extracting part of the code from the big while() statement into a new 
function, IsCertHashFoundInDatabase() would be easier to review (and 
this mistake could have been avoided).

> 
> 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, 3 insertions(+)
> 
> diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index dbfbfcb4fb..74dbffa122 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -908,6 +908,9 @@ IsCertHashFoundInDatabase (
>         goto Done;
>       }
>   
> +    FreePool (HashCtx);
> +    HashCtx = NULL;
> +
>       SiglistHeaderSize = sizeof (EFI_SIGNATURE_LIST) + DbxList->SignatureHeaderSize;
>       CertHash          = (EFI_SIGNATURE_DATA *) ((UINT8 *) DbxList + SiglistHeaderSize);
>       CertHashCount     = (DbxList->SignatureListSize - SiglistHeaderSize) / DbxList->SignatureSize;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>