ShellPkg: CodeQL Fixes.

Includes changes across the pkg 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 <[email protected]> Signed-off-by: Oliver Smith-Denny <[email protected]>
tianocore · Oct 2, 2024 · c875cd5 · c875cd5
1 parent cac73c4
commit c875cd5
Show file tree

Hide file tree

Showing 59 changed files with 878 additions and 236 deletions.
diff --git 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
@@ -576,6 +576,10 @@ UefiMain (
 
     Size       = 100;
     TempString = AllocateZeroPool (Size);
+    if (TempString == NULL) {
+      ASSERT (TempString != NULL);
+      return EFI_OUT_OF_RESOURCES;
+    }
 
     UnicodeSPrint (TempString, Size, L"%d", PcdGet8 (PcdShellSupportLevel));
     Status = InternalEfiShellSetEnv (L"uefishellsupport", TempString, TRUE);
@@ -1326,7 +1330,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 +2613,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 +3339,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
@@ -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
@@ -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,6 +1326,10 @@ UpdateStdInStdOutStdErr (
             TempHandle = CreateFileInterfaceFile (TempHandle, FALSE);
           }
 
+          if (TempHandle == NULL) {
+            return EFI_OUT_OF_RESOURCES;
+          }
+
           ShellParameters->StdIn = TempHandle;
           gST->ConIn             = CreateSimpleTextInOnFile (TempHandle, &gST->ConsoleInHandle);
         }