Browse Source

ShellPkg: Fix conditionally uninitialized variables

Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
Michael Kubacki 1 year ago
parent
commit
11dd44dfbe

+ 1 - 0
ShellPkg/Application/Shell/Shell.c

@@ -1300,6 +1300,7 @@ DoStartupScript (
   CHAR16         *FullFileStringPath;
   UINTN          NewSize;
 
+  CalleeStatus    = EFI_SUCCESS;
   Key.UnicodeChar = CHAR_NULL;
   Key.ScanCode    = 0;
 

+ 31 - 29
ShellPkg/Application/Shell/ShellProtocol.c

@@ -735,50 +735,52 @@ EfiShellGetDeviceName (
     //
     // Now check the parent controller using this as the child.
     //
-    if (DeviceNameToReturn == NULL) {
-      PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentControllerCount, &ParentControllerBuffer);
+    Status = PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentControllerCount, &ParentControllerBuffer);
+    if ((DeviceNameToReturn == NULL) && !EFI_ERROR (Status)) {
       for (LoopVar = 0; LoopVar < ParentControllerCount; LoopVar++) {
-        PARSE_HANDLE_DATABASE_UEFI_DRIVERS (ParentControllerBuffer[LoopVar], &ParentDriverCount, &ParentDriverBuffer);
-        for (HandleCount = 0; HandleCount < ParentDriverCount; HandleCount++) {
-          //
-          // try using that driver's component name with controller and our driver as the child.
-          //
-          Status = gBS->OpenProtocol (
-                          ParentDriverBuffer[HandleCount],
-                          &gEfiComponentName2ProtocolGuid,
-                          (VOID **)&CompName2,
-                          gImageHandle,
-                          NULL,
-                          EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                          );
-          if (EFI_ERROR (Status)) {
+        Status = PARSE_HANDLE_DATABASE_UEFI_DRIVERS (ParentControllerBuffer[LoopVar], &ParentDriverCount, &ParentDriverBuffer);
+        if (!EFI_ERROR (Status)) {
+          for (HandleCount = 0; HandleCount < ParentDriverCount; HandleCount++) {
+            //
+            // try using that driver's component name with controller and our driver as the child.
+            //
             Status = gBS->OpenProtocol (
                             ParentDriverBuffer[HandleCount],
-                            &gEfiComponentNameProtocolGuid,
+                            &gEfiComponentName2ProtocolGuid,
                             (VOID **)&CompName2,
                             gImageHandle,
                             NULL,
                             EFI_OPEN_PROTOCOL_GET_PROTOCOL
                             );
-          }
+            if (EFI_ERROR (Status)) {
+              Status = gBS->OpenProtocol (
+                              ParentDriverBuffer[HandleCount],
+                              &gEfiComponentNameProtocolGuid,
+                              (VOID **)&CompName2,
+                              gImageHandle,
+                              NULL,
+                              EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                              );
+            }
 
-          if (EFI_ERROR (Status)) {
-            continue;
+            if (EFI_ERROR (Status)) {
+              continue;
+            }
+
+            Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);
+            Status = CompName2->GetControllerName (CompName2, ParentControllerBuffer[LoopVar], DeviceHandle, Lang, &DeviceNameToReturn);
+            FreePool (Lang);
+            Lang = NULL;
+            if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
+              break;
+            }
           }
 
-          Lang   = GetBestLanguageForDriver (CompName2->SupportedLanguages, Language, FALSE);
-          Status = CompName2->GetControllerName (CompName2, ParentControllerBuffer[LoopVar], DeviceHandle, Lang, &DeviceNameToReturn);
-          FreePool (Lang);
-          Lang = NULL;
+          SHELL_FREE_NON_NULL (ParentDriverBuffer);
           if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
             break;
           }
         }
-
-        SHELL_FREE_NON_NULL (ParentDriverBuffer);
-        if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) {
-          break;
-        }
       }
 
       SHELL_FREE_NON_NULL (ParentControllerBuffer);

+ 30 - 26
ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c

@@ -1399,10 +1399,11 @@ ShellCommandCreateInitialMappingsAndPaths (
   CHAR16                    *MapName;
   SHELL_MAP_LIST            *MapListItem;
 
-  SplitCurDir = NULL;
-  MapName     = NULL;
-  MapListItem = NULL;
-  HandleList  = NULL;
+  ConsistMappingTable = NULL;
+  SplitCurDir         = NULL;
+  MapName             = NULL;
+  MapListItem         = NULL;
+  HandleList          = NULL;
 
   //
   // Reset the static members back to zero
@@ -1458,32 +1459,35 @@ ShellCommandCreateInitialMappingsAndPaths (
     //
     PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
 
-    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
-    //
-    // Assign new Mappings to all...
-    //
-    for (Count = 0; HandleList[Count] != NULL; Count++) {
-      //
-      // Get default name first
-      //
-      NewDefaultName = ShellCommandCreateNewMappingName (MappingTypeFileSystem);
-      ASSERT (NewDefaultName != NULL);
-      Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, DevicePathList[Count], 0, TRUE);
-      ASSERT_EFI_ERROR (Status);
-      FreePool (NewDefaultName);
-
+    if (!EFI_ERROR (ShellCommandConsistMappingInitialize (&ConsistMappingTable))) {
       //
-      // Now do consistent name
+      // Assign new Mappings to all...
       //
-      NewConsistName = ShellCommandConsistMappingGenMappingName (DevicePathList[Count], ConsistMappingTable);
-      if (NewConsistName != NULL) {
-        Status = ShellCommandAddMapItemAndUpdatePath (NewConsistName, DevicePathList[Count], 0, FALSE);
+      for (Count = 0; HandleList[Count] != NULL; Count++) {
+        //
+        // Get default name first
+        //
+        NewDefaultName = ShellCommandCreateNewMappingName (MappingTypeFileSystem);
+        ASSERT (NewDefaultName != NULL);
+        Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, DevicePathList[Count], 0, TRUE);
         ASSERT_EFI_ERROR (Status);
-        FreePool (NewConsistName);
+        FreePool (NewDefaultName);
+
+        //
+        // Now do consistent name
+        //
+        NewConsistName = ShellCommandConsistMappingGenMappingName (DevicePathList[Count], ConsistMappingTable);
+        if (NewConsistName != NULL) {
+          Status = ShellCommandAddMapItemAndUpdatePath (NewConsistName, DevicePathList[Count], 0, FALSE);
+          ASSERT_EFI_ERROR (Status);
+          FreePool (NewConsistName);
+        }
       }
     }
 
-    ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
+    if (ConsistMappingTable != NULL) {
+      ShellCommandConsistMappingUnInitialize (ConsistMappingTable);
+    }
 
     SHELL_FREE_NON_NULL (HandleList);
     SHELL_FREE_NON_NULL (DevicePathList);
@@ -1626,12 +1630,12 @@ ShellCommandUpdateMapping (
     //
     PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare);
 
-    ShellCommandConsistMappingInitialize (&ConsistMappingTable);
+    Status = ShellCommandConsistMappingInitialize (&ConsistMappingTable);
 
     //
     // Assign new Mappings to remainders
     //
-    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL && !EFI_ERROR (Status); Count++) {
+    for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL; Count++) {
       //
       // Skip ones that already have
       //

+ 11 - 7
ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c

@@ -158,7 +158,10 @@ ShellCommandRunDblk (
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
 
-        ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE);
+        if (EFI_ERROR (ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE))) {
+          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", LbaString);
+          ShellStatus = SHELL_INVALID_PARAMETER;
+        }
       }
 
       if (BlockCountString == NULL) {
@@ -169,12 +172,13 @@ ShellCommandRunDblk (
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
 
-        ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE, FALSE);
-        if (BlockCount > 0x10) {
-          BlockCount = 0x10;
-        } else if (BlockCount == 0) {
-          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
-          ShellStatus = SHELL_INVALID_PARAMETER;
+        if (!EFI_ERROR (ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE, FALSE))) {
+          if (BlockCount > 0x10) {
+            BlockCount = 0x10;
+          } else if (BlockCount == 0) {
+            ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockCountString);
+            ShellStatus = SHELL_INVALID_PARAMETER;
+          }
         }
       }
 

+ 6 - 3
ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c

@@ -112,10 +112,13 @@ ShellCommandRunEfiDecompress (
 
         if (ShellStatus == SHELL_SUCCESS) {
           Status = FileHandleGetSize (InFileHandle, &Temp64Bit);
-          ASSERT (Temp64Bit <= (UINT32)(-1));
-          InSize = (UINTN)Temp64Bit;
           ASSERT_EFI_ERROR (Status);
-          InBuffer = AllocateZeroPool (InSize);
+          if (!EFI_ERROR (Status)) {
+            ASSERT (Temp64Bit <= (UINT32)(-1));
+            InSize   = (UINTN)Temp64Bit;
+            InBuffer = AllocateZeroPool (InSize);
+          }
+
           if (InBuffer == NULL) {
             Status = EFI_OUT_OF_RESOURCES;
           } else {

+ 8 - 6
ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c

@@ -508,9 +508,10 @@ ShellCommandRunConnect (
       Count  = ShellCommandLineGetCount (Package);
 
       if (Param1 != NULL) {
-        Status  = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, FALSE);
-        Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
-        if (EFI_ERROR (Status)) {
+        Status = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, FALSE);
+        if (!EFI_ERROR (Status)) {
+          Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+        } else {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param1);
           ShellStatus = SHELL_INVALID_PARAMETER;
         }
@@ -519,9 +520,10 @@ ShellCommandRunConnect (
       }
 
       if (Param2 != NULL) {
-        Status  = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, FALSE);
-        Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
-        if (EFI_ERROR (Status)) {
+        Status = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, FALSE);
+        if (!EFI_ERROR (Status)) {
+          Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+        } else {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"connect", Param2);
           ShellStatus = SHELL_INVALID_PARAMETER;
         }

+ 11 - 6
ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c

@@ -160,12 +160,17 @@ ShellCommandRunDisconnect (
         Param1 = ShellCommandLineGetRawValue (Package, 1);
         Param2 = ShellCommandLineGetRawValue (Package, 2);
         Param3 = ShellCommandLineGetRawValue (Package, 3);
-        ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE);
-        Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate1) : NULL;
-        ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE);
-        Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate2) : NULL;
-        ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE);
-        Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate3) : NULL;
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE))) {
+          Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate1) : NULL;
+        }
+
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE))) {
+          Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate2) : NULL;
+        }
+
+        if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE))) {
+          Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle ((UINTN)Intermediate3) : NULL;
+        }
 
         if ((Param1 != NULL) && (Handle1 == NULL)) {
           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), gShellDriver1HiiHandle, L"disconnect", Param1);

+ 9 - 12
ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c

@@ -438,25 +438,22 @@ ShellCommandRunDrvDiag (
     ControllerHandleStr = ShellCommandLineGetRawValue (Package, 2);
     ChildHandleStr      = ShellCommandLineGetRawValue (Package, 3);
 
-    if (DriverHandleStr == NULL) {
-      Handle1 = NULL;
-    } else {
-      ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, FALSE);
+    if ((DriverHandleStr != NULL) && !EFI_ERROR (ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, FALSE))) {
       Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle1 = NULL;
     }
 
-    if (ControllerHandleStr == NULL) {
-      Handle2 = NULL;
-    } else {
-      ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TRUE, FALSE);
+    if ((ControllerHandleStr != NULL) && !EFI_ERROR (ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TRUE, FALSE))) {
       Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle2 = NULL;
     }
 
-    if (ChildHandleStr == NULL) {
-      Handle3 = NULL;
-    } else {
-      ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, FALSE);
+    if ((ChildHandleStr != NULL) && !EFI_ERROR (ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, FALSE))) {
       Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+    } else {
+      Handle3 = NULL;
     }
 
     Status = DoDiagnostics (