Current implementation skips to check whether the last four characters are digits when the OptionNumber is NULL. Even worse, it may incorrectly return FALSE when OptionNumber is NULL.
The patch fixes it to always check the variable name even OptionNumber is NULL. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <[email protected]> Cc: Ard Biesheuvel <[email protected]> Cc: Laszlo Ersek <[email protected]> --- .../Library/UefiBootManagerLib/BmLoadOption.c | 45 ++++++++++++++-------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c index b0a35058d0..32918caf32 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c @@ -785,6 +785,8 @@ EfiBootManagerIsValidLoadOptionVariableName ( UINTN VariableNameLen; UINTN Index; UINTN Uint; + EFI_BOOT_MANAGER_LOAD_OPTION_TYPE LocalOptionType; + UINT16 LocalOptionNumber; if (VariableName == NULL) { return FALSE; @@ -792,39 +794,52 @@ EfiBootManagerIsValidLoadOptionVariableName ( VariableNameLen = StrLen (VariableName); + // + // Return FALSE when the variable name length is too small. + // if (VariableNameLen <= 4) { return FALSE; } - for (Index = 0; Index < ARRAY_SIZE (mBmLoadOptionName); Index++) { - if ((VariableNameLen - 4 == StrLen (mBmLoadOptionName[Index])) && - (StrnCmp (VariableName, mBmLoadOptionName[Index], VariableNameLen - 4) == 0) + // + // Return FALSE when the variable name doesn't start with Driver/SysPrep/Boot/PlatformRecovery. + // + for (LocalOptionType = 0; LocalOptionType < ARRAY_SIZE (mBmLoadOptionName); LocalOptionType++) { + if ((VariableNameLen - 4 == StrLen (mBmLoadOptionName[LocalOptionType])) && + (StrnCmp (VariableName, mBmLoadOptionName[LocalOptionType], VariableNameLen - 4) == 0) ) { break; } } + if (LocalOptionType == ARRAY_SIZE (mBmLoadOptionName)) { + return FALSE; + } - if (Index == ARRAY_SIZE (mBmLoadOptionName)) { + // + // Return FALSE when the last four characters are not hex digits. + // + LocalOptionNumber = 0; + for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) { + Uint = BmCharToUint (VariableName[Index]); + if (Uint == -1) { + break; + } else { + LocalOptionNumber = (UINT16) Uint + LocalOptionNumber * 0x10; + } + } + if (Index != VariableNameLen) { return FALSE; } if (OptionType != NULL) { - *OptionType = (EFI_BOOT_MANAGER_LOAD_OPTION_TYPE) Index; + *OptionType = LocalOptionType; } if (OptionNumber != NULL) { - *OptionNumber = 0; - for (Index = VariableNameLen - 4; Index < VariableNameLen; Index++) { - Uint = BmCharToUint (VariableName[Index]); - if (Uint == -1) { - break; - } else { - *OptionNumber = (UINT16) Uint + *OptionNumber * 0x10; - } - } + *OptionNumber = LocalOptionNumber; } - return (BOOLEAN) (Index == VariableNameLen); + return TRUE; } /** -- 2.12.2.windows.2 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
