From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web08.96.1668015212335289939 for ; Wed, 09 Nov 2022 09:33:32 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=UfdngKwc; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from localhost.localdomain (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 5364E20C3338; Wed, 9 Nov 2022 09:33:31 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5364E20C3338 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1668015212; bh=WR7bkiVT+G99iV/AcA0NHxWjEi786HRkQl+lKdqmDqo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UfdngKwcvcO2MzUY1Wj0I6PulF+jDkz34p4A6XRZ07+Zlly0SuRok8XJw2l6AYwF0 ol40WrqRry41cl7dGOKifLamZc8XQHsGQo3S9woEsJ3izsFZB9THo5B3WYqiCLC1B0 kAmXSIR45iNNOK0yIcwd1TIi9+v9emg5U5QAOGxQ= From: "Michael Kubacki" To: devel@edk2.groups.io Cc: Erich McMillan , Michael D Kinney , Michael Kubacki , Ray Ni , Zhichao Gao Subject: [PATCH v1 09/12] ShellPkg: Fix conditionally uninitialized variables Date: Wed, 9 Nov 2022 12:32:43 -0500 Message-Id: <20221109173246.174-10-mikuback@linux.microsoft.com> X-Mailer: git-send-email 2.28.0.windows.1 In-Reply-To: <20221109173246.174-1-mikuback@linux.microsoft.com> References: <20221109173246.174-1-mikuback@linux.microsoft.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Kubacki 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 --- ShellPkg/Application/Shell/Shell.c | 2 +- ShellPkg/Application/Shell/ShellProtocol.c | 4 +- ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 56 +++++++= ++++--------- ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c | 18 ++++--- ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c | 9 ++-- ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c | 14 ++--- ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c | 17 +++--- ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c | 21 ++++---= - 8 files changed, 78 insertions(+), 63 deletions(-) diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Sh= ell/Shell.c index df00adfdfa5b..86db2f4ebb6e 100644 --- a/ShellPkg/Application/Shell/Shell.c +++ b/ShellPkg/Application/Shell/Shell.c @@ -1324,7 +1324,7 @@ DoStartupScript ( } =20 Status =3D RunShellCommand (FileStringPath, &CalleeStatus); - if (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit =3D=3D TRUE= ) { + if (!EFI_ERROR (Status) && (ShellInfoObject.ShellInitSettings.BitUni= on.Bits.Exit =3D=3D TRUE)) { ShellCommandRegisterExit (gEfiShellProtocol->BatchIsActive (), (UI= NT64)CalleeStatus); } =20 diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Applic= ation/Shell/ShellProtocol.c index 509eb60e40f4..9183da284fff 100644 --- a/ShellPkg/Application/Shell/ShellProtocol.c +++ b/ShellPkg/Application/Shell/ShellProtocol.c @@ -729,8 +729,8 @@ EfiShellGetDeviceName ( // // Now check the parent controller using this as the child. // - if (DeviceNameToReturn =3D=3D NULL) { - PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentControllerCoun= t, &ParentControllerBuffer); + Status =3D PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, &ParentContr= ollerCount, &ParentControllerBuffer); + if ((DeviceNameToReturn =3D=3D NULL) && !EFI_ERROR (Status)) { for (LoopVar =3D 0; LoopVar < ParentControllerCount; LoopVar++) { PARSE_HANDLE_DATABASE_UEFI_DRIVERS (ParentControllerBuffer[LoopV= ar], &ParentDriverCount, &ParentDriverBuffer); for (HandleCount =3D 0; HandleCount < ParentDriverCount; HandleC= ount++) { diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c b= /ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c index 36cf46fb2c38..4549cbde9b9a 100644 --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c @@ -1399,10 +1399,11 @@ ShellCommandCreateInitialMappingsAndPaths ( CHAR16 *MapName; SHELL_MAP_LIST *MapListItem; =20 - SplitCurDir =3D NULL; - MapName =3D NULL; - MapListItem =3D NULL; - HandleList =3D NULL; + ConsistMappingTable =3D NULL; + SplitCurDir =3D NULL; + MapName =3D NULL; + MapListItem =3D NULL; + HandleList =3D NULL; =20 // // Reset the static members back to zero @@ -1458,32 +1459,35 @@ ShellCommandCreateInitialMappingsAndPaths ( // PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PRO= TOCOL *), DevicePathCompare); =20 - ShellCommandConsistMappingInitialize (&ConsistMappingTable); - // - // Assign new Mappings to all... - // - for (Count =3D 0; HandleList[Count] !=3D NULL; Count++) { + if (!EFI_ERROR (ShellCommandConsistMappingInitialize (&ConsistMappin= gTable))) { // - // Get default name first + // Assign new Mappings to all... // - NewDefaultName =3D ShellCommandCreateNewMappingName (MappingTypeFi= leSystem); - ASSERT (NewDefaultName !=3D NULL); - Status =3D ShellCommandAddMapItemAndUpdatePath (NewDefaultName, De= vicePathList[Count], 0, TRUE); - ASSERT_EFI_ERROR (Status); - FreePool (NewDefaultName); - - // - // Now do consistent name - // - NewConsistName =3D ShellCommandConsistMappingGenMappingName (Devic= ePathList[Count], ConsistMappingTable); - if (NewConsistName !=3D NULL) { - Status =3D ShellCommandAddMapItemAndUpdatePath (NewConsistName, = DevicePathList[Count], 0, FALSE); + for (Count =3D 0; HandleList[Count] !=3D NULL; Count++) { + // + // Get default name first + // + NewDefaultName =3D ShellCommandCreateNewMappingName (MappingType= FileSystem); + ASSERT (NewDefaultName !=3D NULL); + Status =3D ShellCommandAddMapItemAndUpdatePath (NewDefaultName, = DevicePathList[Count], 0, TRUE); ASSERT_EFI_ERROR (Status); - FreePool (NewConsistName); + FreePool (NewDefaultName); + + // + // Now do consistent name + // + NewConsistName =3D ShellCommandConsistMappingGenMappingName (Dev= icePathList[Count], ConsistMappingTable); + if (NewConsistName !=3D NULL) { + Status =3D ShellCommandAddMapItemAndUpdatePath (NewConsistName= , DevicePathList[Count], 0, FALSE); + ASSERT_EFI_ERROR (Status); + FreePool (NewConsistName); + } } } =20 - ShellCommandConsistMappingUnInitialize (ConsistMappingTable); + if (ConsistMappingTable !=3D NULL) { + ShellCommandConsistMappingUnInitialize (ConsistMappingTable); + } =20 SHELL_FREE_NON_NULL (HandleList); SHELL_FREE_NON_NULL (DevicePathList); @@ -1626,12 +1630,12 @@ ShellCommandUpdateMapping ( // PerformQuickSort (DevicePathList, Count, sizeof (EFI_DEVICE_PATH_PRO= TOCOL *), DevicePathCompare); =20 - ShellCommandConsistMappingInitialize (&ConsistMappingTable); + Status =3D ShellCommandConsistMappingInitialize (&ConsistMappingTabl= e); =20 // // Assign new Mappings to remainders // - for (Count =3D 0; !EFI_ERROR (Status) && HandleList[Count] !=3D NULL= && !EFI_ERROR (Status); Count++) { + for (Count =3D 0; !EFI_ERROR (Status) && HandleList[Count] !=3D NULL= ; Count++) { // // Skip ones that already have // diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c b/ShellPk= g/Library/UefiShellDebug1CommandsLib/Dblk.c index 97a4b57a932f..5329b559ba46 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c @@ -158,7 +158,10 @@ ShellCommandRunDblk ( ShellStatus =3D SHELL_INVALID_PARAMETER; } =20 - 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 =3D SHELL_INVALID_PARAMETER; + } } =20 if (BlockCountString =3D=3D NULL) { @@ -169,12 +172,13 @@ ShellCommandRunDblk ( ShellStatus =3D SHELL_INVALID_PARAMETER; } =20 - ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE,= FALSE); - if (BlockCount > 0x10) { - BlockCount =3D 0x10; - } else if (BlockCount =3D=3D 0) { - ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV= ), gShellDebug1HiiHandle, L"dblk", BlockCountString); - ShellStatus =3D SHELL_INVALID_PARAMETER; + if (!EFI_ERROR (ShellConvertStringToUint64 (BlockCountString, &B= lockCount, TRUE, FALSE))) { + if (BlockCount > 0x10) { + BlockCount =3D 0x10; + } else if (BlockCount =3D=3D 0) { + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_I= NV), gShellDebug1HiiHandle, L"dblk", BlockCountString); + ShellStatus =3D SHELL_INVALID_PARAMETER; + } } } =20 diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c = b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c index 8bf23a2076a1..72f8c087cb69 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c @@ -112,10 +112,13 @@ ShellCommandRunEfiDecompress ( =20 if (ShellStatus =3D=3D SHELL_SUCCESS) { Status =3D FileHandleGetSize (InFileHandle, &Temp64Bit); - ASSERT (Temp64Bit <=3D (UINT32)(-1)); - InSize =3D (UINTN)Temp64Bit; ASSERT_EFI_ERROR (Status); - InBuffer =3D AllocateZeroPool (InSize); + if (!EFI_ERROR (Status)) { + ASSERT (Temp64Bit <=3D (UINT32)(-1)); + InSize =3D (UINTN)Temp64Bit; + InBuffer =3D AllocateZeroPool (InSize); + } + if (InBuffer =3D=3D NULL) { Status =3D EFI_OUT_OF_RESOURCES; } else { diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c b/She= llPkg/Library/UefiShellDriver1CommandsLib/Connect.c index d7a133c0c5b4..870c5b0d1da7 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c @@ -508,9 +508,10 @@ ShellCommandRunConnect ( Count =3D ShellCommandLineGetCount (Package); =20 if (Param1 !=3D NULL) { - Status =3D ShellConvertStringToUint64 (Param1, &Intermediate, T= RUE, FALSE); - Handle1 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); - if (EFI_ERROR (Status)) { + Status =3D ShellConvertStringToUint64 (Param1, &Intermediate, TR= UE, FALSE); + if (!EFI_ERROR (Status)) { + Handle1 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); + } else { ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDL= E), gShellDriver1HiiHandle, L"connect", Param1); ShellStatus =3D SHELL_INVALID_PARAMETER; } @@ -519,9 +520,10 @@ ShellCommandRunConnect ( } =20 if (Param2 !=3D NULL) { - Status =3D ShellConvertStringToUint64 (Param2, &Intermediate, T= RUE, FALSE); - Handle2 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); - if (EFI_ERROR (Status)) { + Status =3D ShellConvertStringToUint64 (Param2, &Intermediate, TR= UE, FALSE); + if (!EFI_ERROR (Status)) { + Handle2 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); + } else { ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDL= E), gShellDriver1HiiHandle, L"connect", Param2); ShellStatus =3D SHELL_INVALID_PARAMETER; } diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c b/= ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c index 009ae5282b27..fd49d1f7ceb4 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c @@ -160,12 +160,17 @@ ShellCommandRunDisconnect ( Param1 =3D ShellCommandLineGetRawValue (Package, 1); Param2 =3D ShellCommandLineGetRawValue (Package, 2); Param3 =3D ShellCommandLineGetRawValue (Package, 3); - ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE)= ; - Handle1 =3D Param1 !=3D NULL ? ConvertHandleIndexToHandle ((UINT= N)Intermediate1) : NULL; - ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE)= ; - Handle2 =3D Param2 !=3D NULL ? ConvertHandleIndexToHandle ((UINT= N)Intermediate2) : NULL; - ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE)= ; - Handle3 =3D Param3 !=3D NULL ? ConvertHandleIndexToHandle ((UINT= N)Intermediate3) : NULL; + if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediat= e1, TRUE, FALSE))) { + Handle1 =3D Param1 !=3D NULL ? ConvertHandleIndexToHandle ((UI= NTN)Intermediate1) : NULL; + } + + if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediat= e2, TRUE, FALSE))) { + Handle2 =3D Param2 !=3D NULL ? ConvertHandleIndexToHandle ((UI= NTN)Intermediate2) : NULL; + } + + if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediat= e3, TRUE, FALSE))) { + Handle3 =3D Param3 !=3D NULL ? ConvertHandleIndexToHandle ((UI= NTN)Intermediate3) : NULL; + } =20 if ((Param1 !=3D NULL) && (Handle1 =3D=3D NULL)) { ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDL= E), gShellDriver1HiiHandle, L"disconnect", Param1); diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c b/She= llPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c index c645c9fd6882..8f70d6b6af39 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c @@ -438,25 +438,22 @@ ShellCommandRunDrvDiag ( ControllerHandleStr =3D ShellCommandLineGetRawValue (Package, 2); ChildHandleStr =3D ShellCommandLineGetRawValue (Package, 3); =20 - if (DriverHandleStr =3D=3D NULL) { - Handle1 =3D NULL; - } else { - ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, = FALSE); + if ((DriverHandleStr !=3D NULL) && ShellConvertStringToUint64 (Drive= rHandleStr, &Intermediate, TRUE, FALSE)) { Handle1 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); + } else { + Handle1 =3D NULL; } =20 - if (ControllerHandleStr =3D=3D NULL) { - Handle2 =3D NULL; - } else { - ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TR= UE, FALSE); + if ((ControllerHandleStr !=3D NULL) && ShellConvertStringToUint64 (C= ontrollerHandleStr, &Intermediate, TRUE, FALSE)) { Handle2 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); + } else { + Handle2 =3D NULL; } =20 - if (ChildHandleStr =3D=3D NULL) { - Handle3 =3D NULL; - } else { - ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, F= ALSE); + if ((ChildHandleStr !=3D NULL) && ShellConvertStringToUint64 (ChildH= andleStr, &Intermediate, TRUE, FALSE)) { Handle3 =3D ConvertHandleIndexToHandle ((UINTN)Intermediate); + } else { + Handle3 =3D NULL; } =20 Status =3D DoDiagnostics ( --=20 2.28.0.windows.1