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>

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index 0f2e677c8a6bbe43013314bf366b38aaadd6351e..efad073880ae040a73aa79448c1f57a4091ca657 100644 (file)
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -42,7 +42,7 @@ VOID                         *mBmTxtInExRegistration  = NULL;
 **/\r
 UINTN\r
 BmSizeOfKeyOption (\r
-  EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption\r
+  IN CONST EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption\r
   )\r
 {\r
   return OFFSET_OF (EFI_BOOT_MANAGER_KEY_OPTION, Keys)\r
@@ -61,8 +61,8 @@ BmSizeOfKeyOption (
 **/\r
 BOOLEAN\r
 BmIsKeyOptionValid (\r
-  IN EFI_BOOT_MANAGER_KEY_OPTION     *KeyOption,\r
-  IN UINTN                           KeyOptionSize\r
+  IN CONST EFI_BOOT_MANAGER_KEY_OPTION *KeyOption,\r
+  IN       UINTN                       KeyOptionSize\r
 )\r
 {\r
   UINT16   OptionName[BM_OPTION_NAME_LEN];\r
@@ -158,16 +158,15 @@ BmCollectKeyOptions (
 {\r
   UINTN                        Index;\r
   BM_COLLECT_KEY_OPTIONS_PARAM *Param;\r
-  EFI_BOOT_MANAGER_KEY_OPTION  *KeyOption;\r
+  VOID                         *KeyOption;\r
   UINT16                       OptionNumber;\r
   UINTN                        KeyOptionSize;\r
 \r
   Param = (BM_COLLECT_KEY_OPTIONS_PARAM *) Context;\r
 \r
   if (BmIsKeyOptionVariable (Name, Guid, &OptionNumber)) {\r
-    GetEfiGlobalVariable2 (Name, (VOID**) &KeyOption, &KeyOptionSize);\r
+    GetEfiGlobalVariable2 (Name, &KeyOption, &KeyOptionSize);\r
     ASSERT (KeyOption != NULL);\r
-    KeyOption->OptionNumber = OptionNumber;\r
     if (BmIsKeyOptionValid (KeyOption, KeyOptionSize)) {\r
       Param->KeyOptions = ReallocatePool (\r
                             Param->KeyOptionCount * sizeof (EFI_BOOT_MANAGER_KEY_OPTION),\r
@@ -179,12 +178,13 @@ BmCollectKeyOptions (
       // Insert the key option in order\r
       //\r
       for (Index = 0; Index < Param->KeyOptionCount; Index++) {\r
-        if (KeyOption->OptionNumber < Param->KeyOptions[Index].OptionNumber) {\r
+        if (OptionNumber < Param->KeyOptions[Index].OptionNumber) {\r
           break;\r
         }\r
       }\r
       CopyMem (&Param->KeyOptions[Index + 1], &Param->KeyOptions[Index], (Param->KeyOptionCount - Index) * sizeof (EFI_BOOT_MANAGER_KEY_OPTION));\r
-      CopyMem (&Param->KeyOptions[Index], KeyOption, BmSizeOfKeyOption (KeyOption));\r
+      CopyMem (&Param->KeyOptions[Index], KeyOption, KeyOptionSize);\r
+      Param->KeyOptions[Index].OptionNumber = OptionNumber;\r
       Param->KeyOptionCount++;\r
     }\r
     FreePool (KeyOption);\r