[PATCH v1 09/12] ShellPkg: Fix conditionally uninitialized variables


Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

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

Cc: Erich McMillan <emcmillan@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <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


Gao, Zhichao
 

See comments below:

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Thursday, November 10, 2022 1:33 AM
To: devel@edk2.groups.io
Cc: Erich McMillan <emcmillan@...>; Kinney, Michael D
<michael.d.kinney@...>; Michael Kubacki
<mikuback@...>; Ni, Ray <ray.ni@...>; Gao, Zhichao
<zhichao.gao@...>
Subject: [PATCH v1 09/12] ShellPkg: Fix conditionally uninitialized variables

From: Michael Kubacki <michael.kubacki@...>

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

Cc: Erich McMillan <emcmillan@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <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/Shell/Shell.c
index df00adfdfa5b..86db2f4ebb6e 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1324,7 +1324,7 @@ DoStartupScript (
}

Status = RunShellCommand (FileStringPath, &CalleeStatus);
- if (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE) {
+ if (!EFI_ERROR (Status) &&
+ (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE)) {
Incorrect here. Cannot handle the unsuccess condition. Better to assign the success initial value to Calleestatus and keep the org logic.

ShellCommandRegisterExit (gEfiShellProtocol->BatchIsActive (),
(UINT64)CalleeStatus);
}

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
b/ShellPkg/Application/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 == 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);
Should we cover above function as well?

Others looks good to me.

Thanks,
Zhichao

for (HandleCount = 0; HandleCount < ParentDriverCount;
HandleCount++) { 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;

- 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++) {
+ if (!EFI_ERROR (ShellCommandConsistMappingInitialize
+ (&ConsistMappingTable))) {
//
- // Get default name first
+ // Assign new Mappings to all...
//
- NewDefaultName = ShellCommandCreateNewMappingName
(MappingTypeFileSystem);
- ASSERT (NewDefaultName != NULL);
- Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName,
DevicePathList[Count], 0, TRUE);
- ASSERT_EFI_ERROR (Status);
- FreePool (NewDefaultName);
-
- //
- // Now do consistent name
- //
- 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 97a4b57a932f..5329b559ba46 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 8bf23a2076a1..72f8c087cb69 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 d7a133c0c5b4..870c5b0d1da7 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 009ae5282b27..fd49d1f7ceb4 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 c645c9fd6882..8f70d6b6af39 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) && 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) && 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) && ShellConvertStringToUint64
+ (ChildHandleStr, &Intermediate, TRUE, FALSE)) {
Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+ } else {
+ Handle3 = NULL;
}

Status = DoDiagnostics (
--
2.28.0.windows.1


Michael Kubacki
 

Thanks. I will include this in the v2 series.

On 11/23/2022 9:19 PM, Gao, Zhichao wrote:
See comments below:

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Thursday, November 10, 2022 1:33 AM
To: devel@edk2.groups.io
Cc: Erich McMillan <emcmillan@...>; Kinney, Michael D
<michael.d.kinney@...>; Michael Kubacki
<mikuback@...>; Ni, Ray <ray.ni@...>; Gao, Zhichao
<zhichao.gao@...>
Subject: [PATCH v1 09/12] ShellPkg: Fix conditionally uninitialized variables

From: Michael Kubacki <michael.kubacki@...>

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

Cc: Erich McMillan <emcmillan@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <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/Shell/Shell.c
index df00adfdfa5b..86db2f4ebb6e 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1324,7 +1324,7 @@ DoStartupScript (
}

Status = RunShellCommand (FileStringPath, &CalleeStatus);
- if (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE) {
+ if (!EFI_ERROR (Status) &&
+ (ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit == TRUE)) {
Incorrect here. Cannot handle the unsuccess condition. Better to assign the success initial value to Calleestatus and keep the org logic.

ShellCommandRegisterExit (gEfiShellProtocol->BatchIsActive (),
(UINT64)CalleeStatus);
}

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
b/ShellPkg/Application/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 == 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);
Should we cover above function as well?
It was not identified by the query results, but I can add it if we like.

Others looks good to me.
Thanks,
Zhichao

for (HandleCount = 0; HandleCount < ParentDriverCount;
HandleCount++) { 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;

- 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++) {
+ if (!EFI_ERROR (ShellCommandConsistMappingInitialize
+ (&ConsistMappingTable))) {
//
- // Get default name first
+ // Assign new Mappings to all...
//
- NewDefaultName = ShellCommandCreateNewMappingName
(MappingTypeFileSystem);
- ASSERT (NewDefaultName != NULL);
- Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName,
DevicePathList[Count], 0, TRUE);
- ASSERT_EFI_ERROR (Status);
- FreePool (NewDefaultName);
-
- //
- // Now do consistent name
- //
- 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 97a4b57a932f..5329b559ba46 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 8bf23a2076a1..72f8c087cb69 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 d7a133c0c5b4..870c5b0d1da7 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 009ae5282b27..fd49d1f7ceb4 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 c645c9fd6882..8f70d6b6af39 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) && 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) && 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) && ShellConvertStringToUint64
+ (ChildHandleStr, &Intermediate, TRUE, FALSE)) {
Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate);
+ } else {
+ Handle3 = NULL;
}

Status = DoDiagnostics (
--
2.28.0.windows.1