Re: [Patch 1/2] SecurityPkg OpalPasswordDxe: Clean password buffer.

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Dong, Eric
> Sent: Wednesday, November 16, 2016 2:15 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [Patch 1/2] SecurityPkg OpalPasswordDxe: Clean password buffer.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c |  1 +
>  SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c    | 17
> +++++++++++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
> index 718d49e..0a32ee2 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
> @@ -240,6 +240,7 @@ OpalDriverPopUpHddPassword (
>    }
> 
>    UnicodeStrToAsciiStrS (Unicode, Ascii, MAX_PASSWORD_SIZE + 1);
> +  ZeroMem (Unicode, sizeof (Unicode));
> 
>    return Ascii;
>  }
> diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
> index 5e3106a..5937ce2 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
> @@ -694,6 +694,8 @@ HiiPsidRevert(
>      Ret = OpalSupportPsidRevert(&Session, Psid.Psid,
> (UINT32)sizeof(Psid.Psid), OpalDisk->OpalDevicePath);
>    }
> 
> +  ZeroMem (Psid.Psid, PSID_CHARACTER_LENGTH);
> +
>    if (Ret == TcgResultSuccess) {
>      AsciiSPrint( Response, DEFAULT_RESPONSE_SIZE, "%a", "PSID Revert:
> Success" );
>    } else {
> @@ -1099,8 +1101,8 @@ HiiPasswordEntered(
>    EFI_STRING_ID            Str
>    )
>  {
> -  OPAL_DISK*                    OpalDisk;
> -  CHAR8
> Password[MAX_PASSWORD_CHARACTER_LENGTH + 1];
> +  OPAL_DISK*                   OpalDisk;
> +  CHAR8
> Password[MAX_PASSWORD_CHARACTER_LENGTH + 1];
>    CHAR16*                      UniStr;
>    UINT32                       PassLength;
>    EFI_STATUS                   Status;
> @@ -1124,15 +1126,20 @@ HiiPasswordEntered(
>    if (UniStr == NULL) {
>      return EFI_NOT_FOUND;
>    }
> +
> +  HiiSetString(gHiiPackageListHandle, Str, L"", NULL);
> +
>    PassLength = (UINT32) StrLen (UniStr);
>    if (PassLength >= sizeof(Password)) {
>      HiiSetFormString(STRING_TOKEN(STR_ACTION_STATUS), "Password
> too long");
> -    gBS->FreePool(UniStr);
> +    ZeroMem (UniStr, StrSize (UniStr));
> +    FreePool(UniStr);
>      return EFI_BUFFER_TOO_SMALL;
>    }
> 
>    UnicodeStrToAsciiStrS (UniStr, Password, sizeof (Password));
> -  gBS->FreePool(UniStr);
> +  ZeroMem (UniStr, StrSize (UniStr));
> +  FreePool(UniStr);
> 
>    if (gHiiConfiguration.SelectedAction == HII_KEY_ID_GOTO_UNLOCK) {
>      Status = HiiUnlock (OpalDisk, Password, PassLength);
> @@ -1154,6 +1161,8 @@ HiiPasswordEntered(
>      Status = HiiSetPassword(OpalDisk, Password, PassLength);
>    }
> 
> +  ZeroMem (Password, sizeof (Password));
> +
>    OpalHiiSetBrowserData ();
> 
>    return Status;
> --
> 2.6.4.windows.1