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>

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));

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);
       }
 
-      //
+      if (CommandWithPath == NULL) {
-      // This should be impossible now.
+        //
-      //
+        // This should be impossible now.
-      ASSERT (CommandWithPath != NULL);
+        //
+        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;
+  UINTN  WalkChar;
-  UINT32  WalkStr;
+  UINTN  WalkStr;
 
   for (WalkStr = 0; WalkStr < StrLen (String); WalkStr++) {
     if (String[WalkStr] == EscapeCharacter) {

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);
+      if (FileDevPath == NULL) {
-      Status      = InternalOpenFileDevicePath (DevPath, &FileHandle, EFI_FILE_MODE_READ, 0);
+        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;

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);
+        if (TempHandle == NULL) {
-        ShellParameters->StdOut = TempHandle;
+          ASSERT (TempHandle != NULL);
-        gST->ConOut             = CreateSimpleTextOutOnFile (TempHandle, &gST->ConsoleOutHandle, gST->ConOut);
+          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);
+        if (TempHandle == NULL) {
-        ShellParameters->StdErr = TempHandle;
+          ASSERT (TempHandle != NULL);
-        gST->StdErr             = CreateSimpleTextOutOnFile (TempHandle, &gST->StandardErrorHandle, gST->StdErr);
+          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;
+          if (TempHandle == NULL) {
-          gST->ConIn             = CreateSimpleTextInOnFile (TempHandle, &gST->ConsoleInHandle);
+            Status = EFI_OUT_OF_RESOURCES;
+          } else {
+            ShellParameters->StdIn = TempHandle;
+            gST->ConIn             = CreateSimpleTextInOnFile (TempHandle, &gST->ConsoleInHandle);
+          }
         }
       }
     }

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);
+    if (Temp != NULL) {
-    Temp = ConvertDevicePathToText (DevPath, TRUE, TRUE);
+      FreePool (Temp);
-    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);
+      FileInfo = FileHandleGetInfo (FileHandle);
-      NewShellNode = CreateAndPopulateShellFileInfo (
+      if (FileInfo != NULL) {
-                       MapName,
+        NewShellNode = CreateAndPopulateShellFileInfo (
-                       EFI_SUCCESS,
+                         MapName,
-                       L"\\",
+                         EFI_SUCCESS,
-                       FileHandle,
+                         L"\\",
-                       FileInfo
+                         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);
     }