Re: [PATCH] ShellPkg/UefiShellLevel3CommandsLib: fix string over-read

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 1/23/2018 10:14 AM, Jian J Wang wrote:
> In the for-loop condition of original code, the expression
> 
>    *CurrentCommand != CHAR_NULL
> 
> is put before expression
> 
>    CurrentCommand < SortedCommandList + SortedCommandListSize/sizeof(CHAR16)
> 
> When CurrentCommand walks to the end of string buffer, one more character
> over the end of string buffer will be read and then stop.
> 
> To fix this issue, just move the last expression to the first one. Because
> of short-circuit evaludation of and-expression, the following one
> 
>    *CurrentCommand != CHAR_NULL
> 
> will not be evaluated if the expression before it is evaludated as FALSE.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>   ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c b/ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c
> index a71ade3a20..75e3d74200 100644
> --- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c
> +++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c
> @@ -397,7 +397,7 @@ ShellCommandRunHelp (
>           CopyListOfCommandNamesWithDynamic(&SortedCommandList, &SortedCommandListSize);
>   
>           for (CurrentCommand = SortedCommandList
> -          ; CurrentCommand != NULL && *CurrentCommand != CHAR_NULL && CurrentCommand < SortedCommandList + SortedCommandListSize/sizeof(CHAR16)
> +          ; CurrentCommand < SortedCommandList + SortedCommandListSize/sizeof(CHAR16) && CurrentCommand != NULL && *CurrentCommand != CHAR_NULL
>             ; CurrentCommand += StrLen(CurrentCommand) + 1
>             ) {
>             //
> 
How about keep "CurrentCommand != NULL" in the beginning? I agree to
swap the other two checks.

-- 
Thanks,
Ray