ShellPkg: Refine code to use Strn**S safe functions instead of Str**S ones in some cases.

Safe string functions may ASSERT when the source length is larger than the MaxDest. This patch use Strn**S to indicate the copy length. 

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <shumin.qiu@intel.com>
Reviewed-by: Tapan Shah <<tapandshah@hp.com>>

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

5 files changed, 45 insertions, 40 deletions

diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 8a71502459..18b6c3e768 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -509,7 +509,7 @@ FileInterfaceStdInRead(
         if (StrStr(CurrentString + TabPos, L":") == NULL) {

           Cwd = ShellInfoObject.NewEfiShellProtocol->GetCurDir(NULL);

           if (Cwd != NULL) {

-            StrCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), Cwd);

+            StrnCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), Cwd, (*BufferSize)/sizeof(CHAR16) - 1);

             if (TabStr[StrLen(TabStr)-1] == L'\\' && *(CurrentString + TabPos) == L'\\' ) {

               TabStr[StrLen(TabStr)-1] = CHAR_NULL;

             }

@@ -523,7 +523,7 @@ FileInterfaceStdInRead(
             StrnCatS(TabStr, (*BufferSize)/sizeof(CHAR16), CurrentString + TabPos, StringLen - TabPos);

           }

         } else {

-          StrCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), CurrentString + TabPos);

+          StrnCpyS(TabStr, (*BufferSize)/sizeof(CHAR16), CurrentString + TabPos, (*BufferSize)/sizeof(CHAR16) - 1);

         }

         StrnCatS(TabStr, (*BufferSize)/sizeof(CHAR16), L"*", (*BufferSize)/sizeof(CHAR16) - 1 - StrLen(TabStr));

         FoundFileList = NULL;

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 779bb53df6..78a64de19e 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -2564,6 +2564,7 @@ RunScriptFileHandle (
   EFI_STATUS          Status;

   SCRIPT_FILE         *NewScriptFile;

   UINTN               LoopVar;

+  UINTN               PrintBuffSize;

   CHAR16              *CommandLine;

   CHAR16              *CommandLine2;

   CHAR16              *CommandLine3;

@@ -2578,6 +2579,7 @@ RunScriptFileHandle (
   ASSERT(!ShellCommandGetScriptExit());

 

   PreScriptEchoState = ShellCommandGetEchoState();

+  PrintBuffSize = PcdGet16(PcdShellPrintBufferSize);

 

   NewScriptFile = (SCRIPT_FILE*)AllocateZeroPool(sizeof(SCRIPT_FILE));

   if (NewScriptFile == NULL) {

@@ -2652,12 +2654,12 @@ RunScriptFileHandle (
   //

   // Now enumerate through the commands and run each one.

   //

-  CommandLine = AllocateZeroPool(PcdGet16(PcdShellPrintBufferSize));

+  CommandLine = AllocateZeroPool(PrintBuffSize);

   if (CommandLine == NULL) {

     DeleteScriptFileStruct(NewScriptFile);

     return (EFI_OUT_OF_RESOURCES);

   }

-  CommandLine2 = AllocateZeroPool(PcdGet16(PcdShellPrintBufferSize));

+  CommandLine2 = AllocateZeroPool(PrintBuffSize);

   if (CommandLine2 == NULL) {

     FreePool(CommandLine);

     DeleteScriptFileStruct(NewScriptFile);

@@ -2669,9 +2671,10 @@ RunScriptFileHandle (
       ; // conditional increment in the body of the loop

   ){

     ASSERT(CommandLine2 != NULL);

-    StrCpyS( CommandLine2, 

-              PcdGet16(PcdShellPrintBufferSize)/sizeof(CHAR16), 

-              NewScriptFile->CurrentCommand->Cl

+    StrnCpyS( CommandLine2, 

+              PrintBuffSize/sizeof(CHAR16), 

+              NewScriptFile->CurrentCommand->Cl,

+              PrintBuffSize/sizeof(CHAR16) - 1

               );

 

     //

@@ -2693,9 +2696,10 @@ RunScriptFileHandle (
       //

       // Due to variability in starting the find and replace action we need to have both buffers the same.

       //

-      StrCpyS( CommandLine, 

-                PcdGet16(PcdShellPrintBufferSize)/sizeof(CHAR16), 

-                CommandLine2

+      StrnCpyS( CommandLine, 

+                PrintBuffSize/sizeof(CHAR16), 

+                CommandLine2,

+                PrintBuffSize/sizeof(CHAR16) - 1

                 );

 

       //

@@ -2704,53 +2708,54 @@ RunScriptFileHandle (
       if (NewScriptFile->Argv != NULL) {

         switch (NewScriptFile->Argc) {

           default:

-            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%9", NewScriptFile->Argv[9], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%9", NewScriptFile->Argv[9], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 9:

-            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%8", NewScriptFile->Argv[8], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%8", NewScriptFile->Argv[8], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 8:

-            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%7", NewScriptFile->Argv[7], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%7", NewScriptFile->Argv[7], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 7:

-            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%6", NewScriptFile->Argv[6], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%6", NewScriptFile->Argv[6], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 6:

-            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%5", NewScriptFile->Argv[5], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%5", NewScriptFile->Argv[5], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 5:

-            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%4", NewScriptFile->Argv[4], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%4", NewScriptFile->Argv[4], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 4:

-            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%3", NewScriptFile->Argv[3], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%3", NewScriptFile->Argv[3], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 3:

-            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%2", NewScriptFile->Argv[2], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%2", NewScriptFile->Argv[2], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 2:

-            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%1", NewScriptFile->Argv[1], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%1", NewScriptFile->Argv[1], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

           case 1:

-            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%0", NewScriptFile->Argv[0], FALSE, TRUE);

+            Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%0", NewScriptFile->Argv[0], FALSE, TRUE);

             ASSERT_EFI_ERROR(Status);

             break;

           case 0:

             break;

         }

       }

-      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%1", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%2", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%3", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%4", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%5", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%6", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%7", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PcdGet16 (PcdShellPrintBufferSize), L"%8", L"\"\"", FALSE, FALSE);

-      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PcdGet16 (PcdShellPrintBufferSize), L"%9", L"\"\"", FALSE, FALSE);

-

-      StrCpyS( CommandLine2, 

-                PcdGet16(PcdShellPrintBufferSize)/sizeof(CHAR16), 

-                CommandLine

+      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%1", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%2", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%3", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%4", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%5", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%6", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%7", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine,  CommandLine2, PrintBuffSize, L"%8", L"\"\"", FALSE, FALSE);

+      Status = ShellCopySearchAndReplace(CommandLine2,  CommandLine, PrintBuffSize, L"%9", L"\"\"", FALSE, FALSE);

+

+      StrnCpyS( CommandLine2, 

+                PrintBuffSize/sizeof(CHAR16), 

+                CommandLine,

+                PrintBuffSize/sizeof(CHAR16) - 1

                 );

 

       LastCommand = NewScriptFile->CurrentCommand;

diff --git a/ShellPkg/Library/UefiDpLib/DpUtilities.c b/ShellPkg/Library/UefiDpLib/DpUtilities.c
index 7290285e23..063eb658c8 100644
--- a/ShellPkg/Library/UefiDpLib/DpUtilities.c
+++ b/ShellPkg/Library/UefiDpLib/DpUtilities.c
@@ -261,7 +261,7 @@ GetNameFromHandle (
                                );

     if (!EFI_ERROR (Status)) {

       SHELL_FREE_NON_NULL (PlatformLanguage);

-      StrCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, StringPtr);

+      StrnCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, StringPtr, DP_GAUGE_STRING_LENGTH);

       mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;

       return;

     }

@@ -305,7 +305,7 @@ GetNameFromHandle (
         //

         // Method 3. Get the name string from FFS UI section

         //

-        StrCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, NameString);

+        StrnCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, NameString, DP_GAUGE_STRING_LENGTH);

         mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;

         FreePool (NameString);

       } else {

@@ -321,7 +321,7 @@ GetNameFromHandle (
       //

       NameString = ConvertDevicePathToText (LoadedImageDevicePath, TRUE, FALSE);

       if (NameString != NULL) {

-        StrCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, NameString);

+        StrnCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, NameString, DP_GAUGE_STRING_LENGTH);

         mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;

         FreePool (NameString);

         return;

@@ -334,7 +334,7 @@ GetNameFromHandle (
   //

   StringPtr = HiiGetString (gDpHiiHandle, STRING_TOKEN (STR_DP_ERROR_NAME), NULL);

   ASSERT (StringPtr != NULL);

-  StrCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, StringPtr);

+  StrnCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, StringPtr, DP_GAUGE_STRING_LENGTH);

   FreePool (StringPtr);

 }

 

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index d818b9b8d4..ac6d0bdb55 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -406,7 +406,7 @@ CascadeProcessVariables (
     FoundVarName = AllocateZeroPool (NameSize);

     if (FoundVarName != NULL) {

       if (PrevName != NULL) {

-        StrCpyS(FoundVarName, NameSize/sizeof(CHAR16), PrevName);

+        StrnCpyS(FoundVarName, NameSize/sizeof(CHAR16), PrevName, NameSize/sizeof(CHAR16) - 1);

       }

 

       Status = gRT->GetNextVariableName (&NameSize, FoundVarName, &FoundVarGuid);

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
index dd878c4cb7..759f486a32 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
@@ -3229,8 +3229,8 @@ QueryTable (
     //

     if ((High > Low && Key >= Low && Key <= High) 

       || (Table[Index].Key == Key)) {

-      StrCpyS (Info, InfoLen, Table[Index].Info);

-      StrCatS (Info, InfoLen, L"\n");

+      StrnCpyS (Info, InfoLen, Table[Index].Info, InfoLen - 1);

+      StrnCatS (Info, InfoLen, L"\n", InfoLen - 1 - StrLen(Info));

       return Key;

     }

   }