ShellPkg: Refactor string manipulation in UefiShellLib command

This patch replaces StrCpy with StrnCpy or refactors out the usage of StrCpy through some other means.
This patch replaces StrCat with StrnCat or refactors out the usage of StrCat through some other means.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
Reviewed-by: Erik Bjorge <erik.c.bjorge@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15839 6f19259b-4bc3-4df7-8a09-765794883524

1 files changed, 40 insertions, 38 deletions

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index ad8e80ea83..819c9f03ac 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -1430,27 +1430,31 @@ InternalShellConvertFileListType (
     //

     // allocate new space to copy strings and structure

     //

-    NewInfo->FullName     = AllocateZeroPool(StrSize(OldInfo->FullName));

-    NewInfo->FileName     = AllocateZeroPool(StrSize(OldInfo->FileName));

-    NewInfo->Info         = AllocateZeroPool((UINTN)OldInfo->Info->Size);

+    NewInfo->FullName     = AllocateCopyPool(StrSize(OldInfo->FullName), OldInfo->FullName);

+    NewInfo->FileName     = AllocateCopyPool(StrSize(OldInfo->FileName), OldInfo->FileName);

+    NewInfo->Info         = AllocateCopyPool((UINTN)OldInfo->Info->Size, OldInfo->Info);

 

     //

     // make sure all the memory allocations were sucessful

     //

     if (NULL == NewInfo->FullName || NewInfo->FileName == NULL || NewInfo->Info == NULL) {

+      //

+      // Free the partially allocated new node

+      //

+      SHELL_FREE_NON_NULL(NewInfo->FullName);

+      SHELL_FREE_NON_NULL(NewInfo->FileName);

+      SHELL_FREE_NON_NULL(NewInfo->Info);

+      SHELL_FREE_NON_NULL(NewInfo);

+

+      //

+      // Free the previously converted stuff

+      //

       ShellCloseFileMetaArg((EFI_SHELL_FILE_INFO**)(&ListHead));

       ListHead = NULL;

       break;

     }

 

     //

-    // Copt the strings and structure

-    //

-    StrCpy(NewInfo->FullName, OldInfo->FullName);

-    StrCpy(NewInfo->FileName, OldInfo->FileName);

-    gBS->CopyMem (NewInfo->Info, OldInfo->Info, (UINTN)OldInfo->Info->Size);

-

-    //

     // add that to the list

     //

     InsertTailList(ListHead, &NewInfo->Link);

@@ -1671,8 +1675,8 @@ ShellFindFilePath (
     if (TestPath == NULL) {

       return (NULL);

     }

-    StrCpy(TestPath, Path);

-    StrCat(TestPath, FileName);

+    StrnCpy(TestPath, Path, Size/sizeof(CHAR16) - 1);

+    StrnCat(TestPath, FileName, Size/sizeof(CHAR16) - 1 - StrLen(TestPath));

     Status = ShellOpenFileByName(TestPath, &Handle, EFI_FILE_MODE_READ, 0);

     if (!EFI_ERROR(Status)){

       if (FileHandleIsDirectory(Handle) != EFI_SUCCESS) {

@@ -1704,12 +1708,12 @@ ShellFindFilePath (
           *TempChar = CHAR_NULL;

         }

         if (TestPath[StrLen(TestPath)-1] != L'\\') {

-          StrCat(TestPath, L"\\");

+          StrnCat(TestPath, L"\\", Size/sizeof(CHAR16) - 1 - StrLen(TestPath));

         }

         if (FileName[0] == L'\\') {

           FileName++;

         }

-        StrCat(TestPath, FileName);

+        StrnCat(TestPath, FileName, Size/sizeof(CHAR16) - 1 - StrLen(TestPath));

         if (StrStr(Walker, L";") != NULL) {

           Walker = StrStr(Walker, L";") + 1;

         } else {

@@ -1778,9 +1782,9 @@ ShellFindFilePathEx (
     return (NULL);

   }

   for (ExtensionWalker = FileExtension, TempChar2 = (CHAR16*)FileExtension;  TempChar2 != NULL ; ExtensionWalker = TempChar2 + 1){

-    StrCpy(TestPath, FileName);

+    StrnCpy(TestPath, FileName, Size/sizeof(CHAR16) - 1);

     if (ExtensionWalker != NULL) {

-      StrCat(TestPath, ExtensionWalker);

+      StrnCat(TestPath, ExtensionWalker, Size/sizeof(CHAR16) - 1 - StrLen(TestPath));

     }

     TempChar = StrStr(TestPath, L";");

     if (TempChar != NULL) {

@@ -1963,6 +1967,7 @@ InternalCommandLineParse (
   UINTN                         ValueSize;

   UINTN                         Count;

   CONST CHAR16                  *TempPointer;

+  UINTN                         CurrentValueSize;

 

   CurrentItemPackage = NULL;

   GetItemValue = 0;

@@ -2018,13 +2023,12 @@ InternalCommandLineParse (
         *CheckPackage = NULL;

         return (EFI_OUT_OF_RESOURCES);

       }

-      CurrentItemPackage->Name  = AllocateZeroPool(StrSize(Argv[LoopCounter]));

+      CurrentItemPackage->Name  = AllocateCopyPool(StrSize(Argv[LoopCounter]), Argv[LoopCounter]);

       if (CurrentItemPackage->Name == NULL) {

         ShellCommandLineFreeVarList(*CheckPackage);

         *CheckPackage = NULL;

         return (EFI_OUT_OF_RESOURCES);

       }

-      StrCpy(CurrentItemPackage->Name,  Argv[LoopCounter]);

       CurrentItemPackage->Type  = CurrentItemType;

       CurrentItemPackage->OriginalPosition = (UINTN)(-1);

       CurrentItemPackage->Value = NULL;

@@ -2062,30 +2066,32 @@ InternalCommandLineParse (
       // get the item VALUE for a previous flag

       //

       if (StrStr(Argv[LoopCounter], L" ") == NULL) {

-        CurrentItemPackage->Value = ReallocatePool(ValueSize, ValueSize + StrSize(Argv[LoopCounter]) + sizeof(CHAR16), CurrentItemPackage->Value);

+        CurrentValueSize = ValueSize + StrSize(Argv[LoopCounter]) + sizeof(CHAR16);

+        CurrentItemPackage->Value = ReallocatePool(ValueSize, CurrentValueSize, CurrentItemPackage->Value);

         ASSERT(CurrentItemPackage->Value != NULL);

         if (ValueSize == 0) {

-          StrCpy(CurrentItemPackage->Value, Argv[LoopCounter]);

+          StrnCpy(CurrentItemPackage->Value, Argv[LoopCounter], CurrentValueSize/sizeof(CHAR16) - 1);

         } else {

-          StrCat(CurrentItemPackage->Value, L" ");

-          StrCat(CurrentItemPackage->Value, Argv[LoopCounter]);

+          StrnCat(CurrentItemPackage->Value, L" ", CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

+          StrnCat(CurrentItemPackage->Value, Argv[LoopCounter], CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

         }

         ValueSize += StrSize(Argv[LoopCounter]) + sizeof(CHAR16);

       } else {

         //

         // the parameter has spaces.  must be quoted.

         //

-        CurrentItemPackage->Value = ReallocatePool(ValueSize, ValueSize + StrSize(Argv[LoopCounter]) + sizeof(CHAR16) + sizeof(CHAR16) + sizeof(CHAR16), CurrentItemPackage->Value);

+        CurrentValueSize =  ValueSize + StrSize(Argv[LoopCounter]) + sizeof(CHAR16) + sizeof(CHAR16) + sizeof(CHAR16);

+        CurrentItemPackage->Value = ReallocatePool(ValueSize, CurrentValueSize, CurrentItemPackage->Value);

         ASSERT(CurrentItemPackage->Value != NULL);

         if (ValueSize == 0) {

-          StrCpy(CurrentItemPackage->Value, L"\"");

-          StrCat(CurrentItemPackage->Value, Argv[LoopCounter]);

-          StrCat(CurrentItemPackage->Value, L"\"");

+          StrnCpy(CurrentItemPackage->Value, L"\"", CurrentValueSize/sizeof(CHAR16) - 1);

+          StrnCat(CurrentItemPackage->Value, Argv[LoopCounter], CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

+          StrnCat(CurrentItemPackage->Value, L"\"", CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

         } else {

-          StrCat(CurrentItemPackage->Value, L" ");

-          StrCat(CurrentItemPackage->Value, L"\"");

-          StrCat(CurrentItemPackage->Value, Argv[LoopCounter]);

-          StrCat(CurrentItemPackage->Value, L"\"");

+          StrnCat(CurrentItemPackage->Value, L" ", CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

+          StrnCat(CurrentItemPackage->Value, L"\"", CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

+          StrnCat(CurrentItemPackage->Value, Argv[LoopCounter], CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

+          StrnCat(CurrentItemPackage->Value, L"\"", CurrentValueSize/sizeof(CHAR16) - 1 - StrLen(CurrentItemPackage->Value));

        }

         ValueSize += StrSize(Argv[LoopCounter]) + sizeof(CHAR16);

       }

@@ -2113,13 +2119,12 @@ InternalCommandLineParse (
       }

       CurrentItemPackage->Name  = NULL;

       CurrentItemPackage->Type  = TypePosition;

-      CurrentItemPackage->Value = AllocateZeroPool(StrSize(TempPointer));

+      CurrentItemPackage->Value = AllocateCopyPool(StrSize(TempPointer), TempPointer);

       if (CurrentItemPackage->Value == NULL) {

         ShellCommandLineFreeVarList(*CheckPackage);

         *CheckPackage = NULL;

         return (EFI_OUT_OF_RESOURCES);

       }

-      StrCpy(CurrentItemPackage->Value, TempPointer);

       CurrentItemPackage->OriginalPosition = Count++;

       InsertHeadList(*CheckPackage, &CurrentItemPackage->Link);

     } else {

@@ -2127,10 +2132,7 @@ InternalCommandLineParse (
       // this was a non-recognised flag... error!

       //

       if (ProblemParam != NULL) {

-        *ProblemParam = AllocateZeroPool(StrSize(Argv[LoopCounter]));

-        if (*ProblemParam != NULL) {

-          StrCpy(*ProblemParam, Argv[LoopCounter]);

-        }

+        *ProblemParam = AllocateCopyPool(StrSize(Argv[LoopCounter]), Argv[LoopCounter]);

       }

       ShellCommandLineFreeVarList(*CheckPackage);

       *CheckPackage = NULL;

@@ -2597,7 +2599,7 @@ ShellCopySearchAndReplace(
   if (Replace == NULL) {

     return (EFI_OUT_OF_RESOURCES);

   }

-  NewString = SetMem16(NewString, NewSize, CHAR_NULL);

+  NewString = ZeroMem(NewString, NewSize);

   while (*SourceString != CHAR_NULL) {

     //

     // if we find the FindTarget and either Skip == FALSE or Skip  and we

@@ -2612,7 +2614,7 @@ ShellCopySearchAndReplace(
         FreePool(Replace);

         return (EFI_BUFFER_TOO_SMALL);

       }

-      StrCat(NewString, Replace);

+      StrnCat(NewString, Replace, NewSize/sizeof(CHAR16) - 1 - StrLen(NewString));

     } else {

       Size = StrSize(NewString);

       if (Size + sizeof(CHAR16) > NewSize) {