From 11dd44dfbefab2ca0b9160bb5ebe40bc1c70f7c1 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Tue, 8 Nov 2022 15:35:39 -0500 Subject: ShellPkg: Fix conditionally uninitialized variables Fixes CodeQL alerts for CWE-457: https://cwe.mitre.org/data/definitions/457.html Cc: Erich McMillan Cc: Michael D Kinney Cc: Michael Kubacki Cc: Ray Ni Cc: Zhichao Gao Co-authored-by: Erich McMillan Signed-off-by: Michael Kubacki Reviewed-by: Zhichao Gao Reviewed-by: Oliver Smith-Denny --- ShellPkg/Application/Shell/Shell.c | 1 + ShellPkg/Application/Shell/ShellProtocol.c | 60 +++++++++++----------- .../UefiShellCommandLib/UefiShellCommandLib.c | 56 ++++++++++---------- ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c | 18 ++++--- .../UefiShellDebug1CommandsLib/EfiDecompress.c | 9 ++-- .../Library/UefiShellDriver1CommandsLib/Connect.c | 14 ++--- .../UefiShellDriver1CommandsLib/Disconnect.c | 17 +++--- .../Library/UefiShellDriver1CommandsLib/DrvDiag.c | 21 ++++---- 8 files changed, 107 insertions(+), 89 deletions(-) (limited to 'ShellPkg') diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c index 0ae6e14a34..f95c799bb2 100644 --- a/ShellPkg/Application/Shell/Shell.c +++ b/ShellPkg/Application/Shell/Shell.c @@ -1300,6 +1300,7 @@ DoStartupScript ( CHAR16 *FullFileStringPath; UINTN NewSize; + CalleeStatus = EFI_SUCCESS; Key.UnicodeChar = CHAR_NULL; Key.ScanCode = 0; diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c index e6d20ab164..da8c31cb03 100644 --- a/ShellPkg/Application/Shell/ShellProtocol.c +++ b/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); diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c index 36cf46fb2c..4549cbde9b 100644 --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c +++ b/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 // diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c index 97a4b57a93..5329b559ba 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c +++ b/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; + } } } diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c index 8bf23a2076..72f8c087cb 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c +++ b/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 { diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c index d7a133c0c5..870c5b0d1d 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c +++ b/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; } diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c index 009ae5282b..fd49d1f7ce 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c +++ b/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); diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c index c645c9fd68..b845d694b2 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c +++ b/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 ( -- cgit v1.2.3