ShellPkg: Shell: CodeQL Fixes

Includes changes across the module for the following CodeQL rules:
 - cpp/comparison-with-wider-type
 - cpp/overflow-buffer
 - cpp/redundant-null-check-param
 - cpp/uselesstest

Co-authored-by: Taylor Beebe <taylor.d.beebe@gmail.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>

5 files changed, 151 insertions, 39 deletions

diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/Application/Shell/FileHandleWrappers.c
index 2ba9b3401a..f98d86a16f 100644
--- a/ShellPkg/Application/Shell/FileHandleWrappers.c
+++ b/ShellPkg/Application/Shell/FileHandleWrappers.c
@@ -2144,6 +2144,10 @@ FileInterfaceFileWrite (
     // Ascii

     //

     AsciiBuffer = AllocateZeroPool (*BufferSize);

+    if (AsciiBuffer == NULL) {

+      return EFI_OUT_OF_RESOURCES;

+    }

+

     AsciiSPrint (AsciiBuffer, *BufferSize, "%S", Buffer);

     Size   = AsciiStrSize (AsciiBuffer) - 1; // (we dont need the null terminator)

     Status = (((EFI_FILE_PROTOCOL_FILE *)This)->Orig->Write (((EFI_FILE_PROTOCOL_FILE *)This)->Orig, &Size, AsciiBuffer));

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 01b4e37871..173ec8c5d4 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -576,6 +576,11 @@ UefiMain (
 

     Size       = 100;

     TempString = AllocateZeroPool (Size);

+    if (TempString == NULL) {

+      ASSERT (TempString != NULL);

+      Status = EFI_OUT_OF_RESOURCES;

+      goto FreeResources;

+    }

 

     UnicodeSPrint (TempString, Size, L"%d", PcdGet8 (PcdShellSupportLevel));

     Status = InternalEfiShellSetEnv (L"uefishellsupport", TempString, TRUE);

@@ -1326,7 +1331,7 @@ DoStartupScript (
     }

 

     Status = RunShellCommand (FileStringPath, &CalleeStatus);

-    if (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE) {

+    if ((!EFI_ERROR (Status)) && (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE)) {

       ShellCommandRegisterExit (gEfiShellProtocol->BatchIsActive (), (UINT64)CalleeStatus);

     }

 

@@ -2609,10 +2614,15 @@ RunCommandOrFile (
         CommandWithPath = ShellFindFilePathEx (FirstParameter, mExecutableExtensions);

       }

 

-      //

-      // This should be impossible now.

-      //

-      ASSERT (CommandWithPath != NULL);

+      if (CommandWithPath == NULL) {

+        //

+        // This should be impossible now.

+        //

+        ASSERT (CommandWithPath != NULL);

+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_SHELL_NOT_FOUND), ShellInfoObject.HiiHandle, FirstParameter);

+        SetLastError (SHELL_NOT_FOUND);

+        return EFI_NOT_FOUND;

+      }

 

       //

       // Make sure that path is not just a directory (or not found)

@@ -3330,8 +3340,8 @@ FindFirstCharacter (
   IN CONST CHAR16  EscapeCharacter

   )

 {

-  UINT32  WalkChar;

-  UINT32  WalkStr;

+  UINTN  WalkChar;

+  UINTN  WalkStr;

 

   for (WalkStr = 0; WalkStr < StrLen (String); WalkStr++) {

     if (String[WalkStr] == EscapeCharacter) {

diff --git a/ShellPkg/Application/Shell/ShellManParser.c b/ShellPkg/Application/Shell/ShellManParser.c
index 5c823cd7f5..f7082467c7 100644
--- a/ShellPkg/Application/Shell/ShellManParser.c
+++ b/ShellPkg/Application/Shell/ShellManParser.c
@@ -549,6 +549,7 @@ ManFileFindTitleSection (
                                 returned help text.

   @retval EFI_INVALID_PARAMETER HelpText is NULL.

   @retval EFI_INVALID_PARAMETER ManFileName is invalid.

+  @retval EFI_INVALID_PARAMETER Command is invalid.

   @retval EFI_NOT_FOUND         There is no help text available for Command.

 **/

 EFI_STATUS

@@ -600,7 +601,12 @@ ProcessManFile (
   TempString = ShellCommandGetCommandHelp (Command);

   if (TempString != NULL) {

     FileHandle = ConvertEfiFileProtocolToShellHandle (CreateFileInterfaceMem (TRUE), NULL);

-    HelpSize   = StrLen (TempString) * sizeof (CHAR16);

+    if (FileHandle == NULL) {

+      Status = EFI_OUT_OF_RESOURCES;

+      goto Done;

+    }

+

+    HelpSize = StrLen (TempString) * sizeof (CHAR16);

     ShellWriteFile (FileHandle, &HelpSize, TempString);

     ShellSetFilePosition (FileHandle, 0);

     HelpSize  = 0;

@@ -624,8 +630,18 @@ ProcessManFile (
     Status = SearchPathForFile (TempString, &FileHandle);

     if (EFI_ERROR (Status)) {

       FileDevPath = FileDevicePath (NULL, TempString);

-      DevPath     = AppendDevicePath (ShellInfoObject.ImageDevPath, FileDevPath);

-      Status      = InternalOpenFileDevicePath (DevPath, &FileHandle, EFI_FILE_MODE_READ, 0);

+      if (FileDevPath == NULL) {

+        Status = EFI_OUT_OF_RESOURCES;

+        goto Done;

+      }

+

+      DevPath = AppendDevicePath (ShellInfoObject.ImageDevPath, FileDevPath);

+      if (DevPath == NULL) {

+        Status = EFI_OUT_OF_RESOURCES;

+        goto Done;

+      }

+

+      Status = InternalOpenFileDevicePath (DevPath, &FileHandle, EFI_FILE_MODE_READ, 0);

       SHELL_FREE_NON_NULL (FileDevPath);

       SHELL_FREE_NON_NULL (DevPath);

     }

@@ -733,7 +749,12 @@ ProcessManFile (
       }

 

       FileHandle = ConvertEfiFileProtocolToShellHandle (CreateFileInterfaceMem (TRUE), NULL);

-      HelpSize   = StrLen (TempString) * sizeof (CHAR16);

+      if (FileHandle == NULL) {

+        Status = EFI_OUT_OF_RESOURCES;

+        goto Done;

+      }

+

+      HelpSize = StrLen (TempString) * sizeof (CHAR16);

       ShellWriteFile (FileHandle, &HelpSize, TempString);

       ShellSetFilePosition (FileHandle, 0);

       HelpSize  = 0;

diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c b/ShellPkg/Application/Shell/ShellParametersProtocol.c
index 64d67d9f13..8464cbf616 100644
--- a/ShellPkg/Application/Shell/ShellParametersProtocol.c
+++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c
@@ -354,7 +354,11 @@ CreatePopulateInstallShellParametersProtocol (
   Status = SHELL_GET_ENVIRONMENT_VARIABLE (L"ShellOpt", &Size, FullCommandLine);

   if (Status == EFI_BUFFER_TOO_SMALL) {

     FullCommandLine = AllocateZeroPool (Size + LoadedImage->LoadOptionsSize);

-    Status          = SHELL_GET_ENVIRONMENT_VARIABLE (L"ShellOpt", &Size, FullCommandLine);

+    if (FullCommandLine == NULL) {

+      return EFI_OUT_OF_RESOURCES;

+    }

+

+    Status = SHELL_GET_ENVIRONMENT_VARIABLE (L"ShellOpt", &Size, FullCommandLine);

   }

 

   if (Status == EFI_NOT_FOUND) {

@@ -738,6 +742,7 @@ UpdateStdInStdOutStdErr (
   OutAppend        = FALSE;

   CommandLineCopy  = NULL;

   FirstLocation    = NULL;

+  TempHandle       = NULL;

 

   if ((ShellParameters == NULL) || (SystemTableInfo == NULL) || (OldStdIn == NULL) || (OldStdOut == NULL) || (OldStdErr == NULL)) {

     return (EFI_INVALID_PARAMETER);

@@ -1176,7 +1181,10 @@ UpdateStdInStdOutStdErr (
 

         if (!ErrUnicode && !EFI_ERROR (Status)) {

           TempHandle = CreateFileInterfaceFile (TempHandle, FALSE);

-          ASSERT (TempHandle != NULL);

+          if (TempHandle == NULL) {

+            ASSERT (TempHandle != NULL);

+            Status = EFI_OUT_OF_RESOURCES;

+          }

         }

 

         if (!EFI_ERROR (Status)) {

@@ -1223,7 +1231,10 @@ UpdateStdInStdOutStdErr (
 

           if (!OutUnicode && !EFI_ERROR (Status)) {

             TempHandle = CreateFileInterfaceFile (TempHandle, FALSE);

-            ASSERT (TempHandle != NULL);

+            if (TempHandle == NULL) {

+              ASSERT (TempHandle != NULL);

+              Status = EFI_OUT_OF_RESOURCES;

+            }

           }

 

           if (!EFI_ERROR (Status)) {

@@ -1245,9 +1256,13 @@ UpdateStdInStdOutStdErr (
         }

 

         TempHandle = CreateFileInterfaceEnv (StdOutVarName);

-        ASSERT (TempHandle != NULL);

-        ShellParameters->StdOut = TempHandle;

-        gST->ConOut             = CreateSimpleTextOutOnFile (TempHandle, &gST->ConsoleOutHandle, gST->ConOut);

+        if (TempHandle == NULL) {

+          ASSERT (TempHandle != NULL);

+          Status = EFI_OUT_OF_RESOURCES;

+        } else {

+          ShellParameters->StdOut = TempHandle;

+          gST->ConOut             = CreateSimpleTextOutOnFile (TempHandle, &gST->ConsoleOutHandle, gST->ConOut);

+        }

       }

 

       //

@@ -1262,9 +1277,13 @@ UpdateStdInStdOutStdErr (
         }

 

         TempHandle = CreateFileInterfaceEnv (StdErrVarName);

-        ASSERT (TempHandle != NULL);

-        ShellParameters->StdErr = TempHandle;

-        gST->StdErr             = CreateSimpleTextOutOnFile (TempHandle, &gST->StandardErrorHandle, gST->StdErr);

+        if (TempHandle == NULL) {

+          ASSERT (TempHandle != NULL);

+          Status = EFI_OUT_OF_RESOURCES;

+        } else {

+          ShellParameters->StdErr = TempHandle;

+          gST->StdErr             = CreateSimpleTextOutOnFile (TempHandle, &gST->StandardErrorHandle, gST->StdErr);

+        }

       }

 

       //

@@ -1307,8 +1326,12 @@ UpdateStdInStdOutStdErr (
             TempHandle = CreateFileInterfaceFile (TempHandle, FALSE);

           }

 

-          ShellParameters->StdIn = TempHandle;

-          gST->ConIn             = CreateSimpleTextInOnFile (TempHandle, &gST->ConsoleInHandle);

+          if (TempHandle == NULL) {

+            Status = EFI_OUT_OF_RESOURCES;

+          } else {

+            ShellParameters->StdIn = TempHandle;

+            gST->ConIn             = CreateSimpleTextInOnFile (TempHandle, &gST->ConsoleInHandle);

+          }

         }

       }

     }

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index da8c31cb03..ff53f6ba4f 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -436,7 +436,10 @@ EfiShellGetFilePathFromDevicePath (
         if ((DevicePathType (&FilePath->Header) != MEDIA_DEVICE_PATH) ||

             (DevicePathSubType (&FilePath->Header) != MEDIA_FILEPATH_DP))

         {

-          FreePool (PathForReturn);

+          if (PathForReturn != NULL) {

+            FreePool (PathForReturn);

+          }

+

           return NULL;

         }

 

@@ -447,7 +450,10 @@ EfiShellGetFilePathFromDevicePath (
 

         AlignedNode = AllocateCopyPool (DevicePathNodeLength (FilePath), FilePath);

         if (AlignedNode == NULL) {

-          FreePool (PathForReturn);

+          if (PathForReturn != NULL) {

+            FreePool (PathForReturn);

+          }

+

           return NULL;

         }

 

@@ -719,7 +725,11 @@ EfiShellGetDeviceName (
         continue;

       }

 

-      Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);

+      Lang = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);

+      if (Lang == NULL) {

+        continue;

+      }

+

       Status = CompName2->GetControllerName (CompName2, DeviceHandle, NULL, Lang, &DeviceNameToReturn);

       FreePool (Lang);

       Lang = NULL;

@@ -767,7 +777,11 @@ EfiShellGetDeviceName (
               continue;

             }

 

-            Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);

+            Lang = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);

+            if (Lang == NULL) {

+              continue;

+            }

+

             Status = CompName2->GetControllerName (CompName2, ParentControllerBuffer[LoopVar], DeviceHandle, Lang, &DeviceNameToReturn);

             FreePool (Lang);

             Lang = NULL;

@@ -1817,16 +1831,32 @@ EfiShellExecute (
     return (EFI_UNSUPPORTED);

   }

 

+  Temp = NULL;

   if (NestingEnabled ()) {

     DevPath = AppendDevicePath (ShellInfoObject.ImageDevPath, ShellInfoObject.FileDevPath);

+    if (DevPath == NULL) {

+      return EFI_OUT_OF_RESOURCES;

+    }

 

     DEBUG_CODE_BEGIN ();

     Temp = ConvertDevicePathToText (ShellInfoObject.FileDevPath, TRUE, TRUE);

-    FreePool (Temp);

+    if (Temp != NULL) {

+      FreePool (Temp);

+    }

+

     Temp = ConvertDevicePathToText (ShellInfoObject.ImageDevPath, TRUE, TRUE);

-    FreePool (Temp);

-    Temp = ConvertDevicePathToText (DevPath, TRUE, TRUE);

-    FreePool (Temp);

+    if (Temp != NULL) {

+      FreePool (Temp);

+    }

+

+    if (DevPath != NULL) {

+      Temp = ConvertDevicePathToText (DevPath, TRUE, TRUE);

+    }

+

+    if (Temp != NULL) {

+      FreePool (Temp);

+    }

+

     DEBUG_CODE_END ();

 

     Temp = NULL;

@@ -2395,6 +2425,8 @@ ShellSearchHandle (
   CHAR16               *NewFullName;

   UINTN                Size;

 

+  NewShellNode = NULL;

+  FileInfo     = NULL;

   if (  (FilePattern      == NULL)

      || (UnicodeCollation == NULL)

      || (FileList         == NULL)

@@ -2434,14 +2466,17 @@ ShellSearchHandle (
       //

       // We want the root node.  create the node.

       //

-      FileInfo     = FileHandleGetInfo (FileHandle);

-      NewShellNode = CreateAndPopulateShellFileInfo (

-                       MapName,

-                       EFI_SUCCESS,

-                       L"\\",

-                       FileHandle,

-                       FileInfo

-                       );

+      FileInfo = FileHandleGetInfo (FileHandle);

+      if (FileInfo != NULL) {

+        NewShellNode = CreateAndPopulateShellFileInfo (

+                         MapName,

+                         EFI_SUCCESS,

+                         L"\\",

+                         FileHandle,

+                         FileInfo

+                         );

+      }

+

       SHELL_FREE_NON_NULL (FileInfo);

     } else {

       //

@@ -2631,6 +2666,9 @@ EfiShellFindFiles (
   }

 

   PatternCopy = PathCleanUpDirectories (PatternCopy);

+  if (PatternCopy == NULL) {

+    return (EFI_OUT_OF_RESOURCES);

+  }

 

   Count = StrStr (PatternCopy, L":") - PatternCopy + 1;

   ASSERT (Count <= StrLen (PatternCopy));

@@ -2715,6 +2753,10 @@ EfiShellOpenFileList (
   //

   if (StrStr (Path, L":") == NULL) {

     CurDir = EfiShellGetCurDir (NULL);

+    if (CurDir == NULL) {

+      return EFI_NOT_FOUND;

+    }

+

     ASSERT ((Path2 == NULL && Path2Size == 0) || (Path2 != NULL));

     StrnCatGrow (&Path2, &Path2Size, CurDir, 0);

     StrnCatGrow (&Path2, &Path2Size, L"\\", 0);

@@ -2855,6 +2897,10 @@ EfiShellGetEnvEx (
         // Allocate the space and recall the get function

         //

         Buffer = AllocateZeroPool (Size);

+        if (Buffer == NULL) {

+          return NULL;

+        }

+

         Status = SHELL_GET_ENVIRONMENT_VARIABLE_AND_ATTRIBUTES (Name, Attributes, &Size, Buffer);

       }

 

@@ -3122,7 +3168,10 @@ EfiShellSetCurDir (
   }

 

   DirectoryName = StrnCatGrow (&DirectoryName, NULL, Dir, 0);

-  ASSERT (DirectoryName != NULL);

+  if (DirectoryName == NULL) {

+    ASSERT (DirectoryName != NULL);

+    return (EFI_OUT_OF_RESOURCES);

+  }

 

   PathCleanUpDirectories (DirectoryName);

 

@@ -3500,6 +3549,11 @@ EfiShellGetAlias (
     Status  = gRT->GetVariable (AliasLower, &gShellAliasGuid, &Attribs, &RetSize, RetVal);

     if (Status == EFI_BUFFER_TOO_SMALL) {

       RetVal = AllocateZeroPool (RetSize);

+      if (RetVal == NULL) {

+        FreePool (AliasLower);

+        return NULL;

+      }

+

       Status = gRT->GetVariable (AliasLower, &gShellAliasGuid, &Attribs, &RetSize, RetVal);

     }