ShellPkg/Shell: Avoid reading content beyond string boundary

[mirror_edk2.git] / ShellPkg / Application / Shell / ShellEnvVar.c

diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c b/ShellPkg/Application/Shell/ShellEnvVar.c
index 9f87b9074fcc6ca579819862ab39bd5966822844..4c49c1ca980e1df17f9d238974f3ccc46fa040a5 100644 (file)
--- a/ShellPkg/Application/Shell/ShellEnvVar.c
+++ b/ShellPkg/Application/Shell/ShellEnvVar.c
@@ -91,7 +91,6 @@ IsVolatileEnv (
   @param[in] List               The pointer to pointer to list.\r
 **/\r
 VOID\r
-EFIAPI\r
 FreeEnvironmentVariableList(\r
   IN LIST_ENTRY *List\r
   )\r
@@ -128,7 +127,6 @@ FreeEnvironmentVariableList(
   @retval EFI_SUCCESS           the list was created sucessfully.\r
 **/\r
 EFI_STATUS\r
-EFIAPI\r
 GetEnvironmentVariableList(\r
   IN OUT LIST_ENTRY *ListHead\r
   )\r
@@ -180,7 +178,10 @@ GetEnvironmentVariableList(
         Status = EFI_OUT_OF_RESOURCES;\r
       } else {\r
         ValSize = ValBufferSize;\r
-        VarList->Val = AllocateZeroPool(ValSize);\r
+        //\r
+        // We need another CHAR16 to save '\0' in VarList->Val.\r
+        //\r
+        VarList->Val = AllocateZeroPool (ValSize + sizeof (CHAR16));\r
         if (VarList->Val == NULL) {\r
             SHELL_FREE_NON_NULL(VarList);\r
             Status = EFI_OUT_OF_RESOURCES;\r
@@ -190,7 +191,10 @@ GetEnvironmentVariableList(
         if (Status == EFI_BUFFER_TOO_SMALL){\r
           ValBufferSize = ValSize > ValBufferSize * 2 ? ValSize : ValBufferSize * 2;\r
           SHELL_FREE_NON_NULL (VarList->Val);\r
-          VarList->Val = AllocateZeroPool(ValBufferSize);\r
+          //\r
+          // We need another CHAR16 to save '\0' in VarList->Val.\r
+          //\r
+          VarList->Val = AllocateZeroPool (ValBufferSize + sizeof (CHAR16));\r
           if (VarList->Val == NULL) {\r
             SHELL_FREE_NON_NULL(VarList);\r
             Status = EFI_OUT_OF_RESOURCES;\r
@@ -238,7 +242,6 @@ GetEnvironmentVariableList(
   @retval EFI_SUCCESS           the list was Set sucessfully.\r
 **/\r
 EFI_STATUS\r
-EFIAPI\r
 SetEnvironmentVariableList(\r
   IN LIST_ENTRY *ListHead\r
   )\r
@@ -275,7 +278,7 @@ SetEnvironmentVariableList(
       ; !IsNull(ListHead, &Node->Link)\r
       ; Node = (ENV_VAR_LIST*)GetNextNode(ListHead, &Node->Link)\r
      ){\r
-    Size = StrSize(Node->Val);\r
+    Size = StrSize (Node->Val) - sizeof (CHAR16);\r
     if (Node->Atts & EFI_VARIABLE_NON_VOLATILE) {\r
       Status = SHELL_SET_ENVIRONMENT_VARIABLE_NV(Node->Key, Size, Node->Val);\r
     } else {\r
@@ -302,7 +305,6 @@ SetEnvironmentVariableList(
   @sa SetEnvironmentVariableList\r
 **/\r
 EFI_STATUS\r
-EFIAPI\r
 SetEnvironmentVariables(\r
   IN CONST CHAR16 **Environment\r
   )\r
@@ -442,8 +444,11 @@ ShellFindEnvVarInList (
                     including the tailing CHAR_NULL\r
   @param Atts       The attributes of the variable.\r
 \r
+  @retval EFI_SUCCESS  The environment variable was added to list successfully.\r
+  @retval others       Some errors happened.\r
+\r
 **/\r
-VOID\r
+EFI_STATUS\r
 ShellAddEnvVarToList (\r
   IN CONST CHAR16     *Key,\r
   IN CONST CHAR16     *Value,\r
@@ -452,9 +457,16 @@ ShellAddEnvVarToList (
   )\r
 {\r
   ENV_VAR_LIST      *Node;\r
+  CHAR16            *LocalKey;\r
+  CHAR16            *LocalValue;\r
   \r
   if (Key == NULL || Value == NULL || ValueSize == 0) {\r
-    return;\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+\r
+  LocalValue = AllocateCopyPool (ValueSize, Value);\r
+  if (LocalValue == NULL) {\r
+    return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
   //\r
@@ -467,10 +479,8 @@ ShellAddEnvVarToList (
     if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {\r
       Node->Atts = Atts;\r
       SHELL_FREE_NON_NULL(Node->Val);\r
-      Node->Val  = AllocateZeroPool (ValueSize);\r
-      ASSERT (Node->Val != NULL);\r
-      CopyMem(Node->Val, Value, ValueSize);\r
-      return;\r
+      Node->Val  = LocalValue;\r
+      return EFI_SUCCESS;\r
     }\r
   }\r
 \r
@@ -478,16 +488,23 @@ ShellAddEnvVarToList (
   // If the environment varialbe key doesn't exist in list just insert\r
   // a new node.\r
   //\r
+  LocalKey = AllocateCopyPool (StrSize(Key), Key);\r
+  if (LocalKey == NULL) {\r
+    FreePool (LocalValue);\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
   Node = (ENV_VAR_LIST*)AllocateZeroPool (sizeof(ENV_VAR_LIST));\r
-  ASSERT (Node != NULL);\r
-  Node->Key = AllocateCopyPool(StrSize(Key), Key);\r
-  ASSERT (Node->Key != NULL);\r
-  Node->Val = AllocateCopyPool(ValueSize, Value);\r
-  ASSERT (Node->Val != NULL);\r
+  if (Node == NULL) {\r
+    FreePool (LocalKey);\r
+    FreePool (LocalValue);\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+  Node->Key = LocalKey;\r
+  Node->Val = LocalValue;\r
   Node->Atts = Atts;\r
   InsertTailList(&gShellEnvVarList.Link, &Node->Link);\r
 \r
-  return;\r
+  return EFI_SUCCESS;\r
 }\r
 \r
 /**\r