MdeModulePkg/BDS: Fix a buffer overflow bug

KeyOption points to a buffer holding the content of Key####.
So its size is smaller than EFI_BOOT_MANAGER_KEY_OPTION.
Old code to assign value to KeyOption->OptionNumber modifies
the memory outside of the KeyOption buffer.

The patch fixes this bug.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Steven Shi <steven.shi@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jeff Fan <jeff.fan@intel.com>

1 files changed, 8 insertions, 8 deletions

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index 0f2e677c8a..efad073880 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -42,7 +42,7 @@ VOID                         *mBmTxtInExRegistration  = NULL;
 **/

 UINTN

 BmSizeOfKeyOption (

-  EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption

+  IN CONST EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption

   )

 {

   return OFFSET_OF (EFI_BOOT_MANAGER_KEY_OPTION, Keys)

@@ -61,8 +61,8 @@ BmSizeOfKeyOption (
 **/

 BOOLEAN

 BmIsKeyOptionValid (

-  IN EFI_BOOT_MANAGER_KEY_OPTION     *KeyOption,

-  IN UINTN                           KeyOptionSize

+  IN CONST EFI_BOOT_MANAGER_KEY_OPTION *KeyOption,

+  IN       UINTN                       KeyOptionSize

 )

 {

   UINT16   OptionName[BM_OPTION_NAME_LEN];

@@ -158,16 +158,15 @@ BmCollectKeyOptions (
 {

   UINTN                        Index;

   BM_COLLECT_KEY_OPTIONS_PARAM *Param;

-  EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption;

+  VOID                         *KeyOption;

   UINT16                       OptionNumber;

   UINTN                        KeyOptionSize;

 

   Param = (BM_COLLECT_KEY_OPTIONS_PARAM *) Context;

 

   if (BmIsKeyOptionVariable (Name, Guid, &OptionNumber)) {

-    GetEfiGlobalVariable2 (Name, (VOID**) &KeyOption, &KeyOptionSize);

+    GetEfiGlobalVariable2 (Name, &KeyOption, &KeyOptionSize);

     ASSERT (KeyOption != NULL);

-    KeyOption->OptionNumber = OptionNumber;

     if (BmIsKeyOptionValid (KeyOption, KeyOptionSize)) {

       Param->KeyOptions = ReallocatePool (

                             Param->KeyOptionCount * sizeof (EFI_BOOT_MANAGER_KEY_OPTION),

@@ -179,12 +178,13 @@ BmCollectKeyOptions (
       // Insert the key option in order

       //

       for (Index = 0; Index < Param->KeyOptionCount; Index++) {

-        if (KeyOption->OptionNumber < Param->KeyOptions[Index].OptionNumber) {

+        if (OptionNumber < Param->KeyOptions[Index].OptionNumber) {

           break;

         }

       }

       CopyMem (&Param->KeyOptions[Index + 1], &Param->KeyOptions[Index], (Param->KeyOptionCount - Index) * sizeof (EFI_BOOT_MANAGER_KEY_OPTION));

-      CopyMem (&Param->KeyOptions[Index], KeyOption, BmSizeOfKeyOption (KeyOption));

+      CopyMem (&Param->KeyOptions[Index], KeyOption, KeyOptionSize);

+      Param->KeyOptions[Index].OptionNumber = OptionNumber;

       Param->KeyOptionCount++;

     }

     FreePool (KeyOption);