ShellPkg: UefiShellDebug1CommandsLib: CodeQL Fixes

Includes changes across the module 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 <taylor.d.beebe@gmail.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
This commit is contained in:
Oliver Smith-Denny 2024-10-03 10:23:33 -07:00 committed by mergify[bot]
parent 2d10dc1fb5
commit 040afc1e3b
24 changed files with 161 additions and 30 deletions

View File

@ -280,7 +280,13 @@ ShellCommandRunComp (
ShellStatus = SHELL_INVALID_PARAMETER;
} else {
TempParam = ShellCommandLineGetRawValue (Package, 1);
ASSERT (TempParam != NULL);
if (TempParam == NULL) {
ASSERT (TempParam != NULL);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"comp", TempParam);
ShellStatus = SHELL_INVALID_PARAMETER;
return (ShellStatus);
}
FileName1 = ShellFindFilePath (TempParam);
if (FileName1 == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_FILE_FIND_FAIL), gShellDebug1HiiHandle, L"comp", TempParam);
@ -294,7 +300,13 @@ ShellCommandRunComp (
}
TempParam = ShellCommandLineGetRawValue (Package, 2);
ASSERT (TempParam != NULL);
if (TempParam == NULL) {
ASSERT (TempParam != NULL);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"comp", TempParam);
ShellStatus = SHELL_INVALID_PARAMETER;
return (ShellStatus);
}
FileName2 = ShellFindFilePath (TempParam);
if (FileName2 == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_FILE_FIND_FAIL), gShellDebug1HiiHandle, L"comp", TempParam);
@ -367,7 +379,7 @@ ShellCommandRunComp (
}
if (ShellStatus == SHELL_SUCCESS) {
while (DiffPointNumber < DifferentCount) {
while ((UINT64)DiffPointNumber < DifferentCount) {
DataSizeFromFile1 = 1;
DataSizeFromFile2 = 1;
OneByteFromFile1 = 0;

View File

@ -111,6 +111,8 @@ ShellCommandRunDblk (
UINT64 BlockCount;
EFI_DEVICE_PATH_PROTOCOL *DevPath;
Lba = 0;
BlockCount = 0;
ShellStatus = SHELL_SUCCESS;
Status = EFI_SUCCESS;
@ -186,7 +188,7 @@ ShellCommandRunDblk (
//
// do the work if we have a valid block identifier
//
if (gEfiShellProtocol->GetDevicePathFromMap (BlockName) == NULL) {
if ((BlockName == NULL) || (gEfiShellProtocol->GetDevicePathFromMap (BlockName) == NULL)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"dblk", BlockName);
ShellStatus = SHELL_INVALID_PARAMETER;
} else {

View File

@ -448,7 +448,11 @@ CascadeProcessVariables (
StrnCatGrow (&FoundVarName, &NameSize, PrevName, 0);
} else {
FoundVarName = AllocateZeroPool (sizeof (CHAR16));
NameSize = sizeof (CHAR16);
if (FoundVarName == NULL) {
return (SHELL_OUT_OF_RESOURCES);
}
NameSize = sizeof (CHAR16);
}
Status = gRT->GetNextVariableName (&NameSize, FoundVarName, &FoundVarGuid);

View File

@ -101,8 +101,14 @@ ShellCommandRunEdit (
//
if (ShellCommandLineGetCount (Package) == 2) {
TempParam = ShellCommandLineGetRawValue (Package, 1);
ASSERT (TempParam != NULL);
FileBufferSetFileName (TempParam);
if (TempParam == NULL) {
ASSERT (TempParam != NULL);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), gShellDebug1HiiHandle, L"edit");
ShellStatus = SHELL_INVALID_PARAMETER;
} else {
FileBufferSetFileName (TempParam);
}
// if (EFI_ERROR(ShellFileExists(MainEditor.FileBuffer->FileName))) {
// Status = ShellOpenFileByName(MainEditor.FileBuffer->FileName, &TempHandle, EFI_FILE_MODE_CREATE|EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, 0);
// if (!EFI_ERROR(Status)) {

View File

@ -1378,7 +1378,9 @@ MainCommandDisplayHelp (
//
for (CurrentLine = 0; 0 != MainMenuHelpInfo[CurrentLine]; CurrentLine++) {
InfoString = HiiGetString (gShellDebug1HiiHandle, MainMenuHelpInfo[CurrentLine], NULL);
ShellPrintEx (0, CurrentLine+1, L"%E%s%N", InfoString);
if (InfoString != NULL) {
ShellPrintEx (0, CurrentLine+1, L"%E%s%N", InfoString);
}
}
//

View File

@ -107,6 +107,9 @@ MenuBarRefresh (
//
for (Item = MenuItems; Item != NULL && Item->Function != NULL; Item++) {
NameString = HiiGetString (gShellDebug1HiiHandle, Item->NameToken, NULL);
if (NameString == NULL) {
return EFI_INVALID_PARAMETER;
}
Width = MAX ((StrLen (NameString) + 6), 20);
if (((Col + Width) > LastCol)) {
@ -115,6 +118,10 @@ MenuBarRefresh (
}
FunctionKeyString = HiiGetString (gShellDebug1HiiHandle, Item->FunctionKeyToken, NULL);
if (FunctionKeyString == NULL) {
FreePool (NameString);
return EFI_INVALID_PARAMETER;
}
ShellPrintEx ((INT32)(Col) - 1, (INT32)(Row) - 1, L"%E%s%N %H%s%N ", FunctionKeyString, NameString);

View File

@ -79,10 +79,15 @@ ShellCommandRunEfiCompress (
ShellStatus = SHELL_INVALID_PARAMETER;
} else {
TempParam = ShellCommandLineGetRawValue (Package, 1);
ASSERT (TempParam != NULL);
if (TempParam == NULL) {
ASSERT (TempParam != NULL);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"eficompress");
ShellStatus = SHELL_INVALID_PARAMETER;
}
InFileName = ShellFindFilePath (TempParam);
OutFileName = ShellCommandLineGetRawValue (Package, 2);
if (InFileName == NULL) {
if ((InFileName == NULL) || (OutFileName == NULL)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_FILE_FIND_FAIL), gShellDebug1HiiHandle, L"eficompress", TempParam);
ShellStatus = SHELL_NOT_FOUND;
} else {

View File

@ -85,10 +85,16 @@ ShellCommandRunEfiDecompress (
ShellStatus = SHELL_INVALID_PARAMETER;
} else {
TempParam = ShellCommandLineGetRawValue (Package, 1);
ASSERT (TempParam != NULL);
if (TempParam == NULL) {
ASSERT (TempParam != NULL);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"efidecompress");
ShellStatus = SHELL_INVALID_PARAMETER;
return (ShellStatus);
}
InFileName = ShellFindFilePath (TempParam);
OutFileName = ShellCommandLineGetRawValue (Package, 2);
if (InFileName == NULL) {
if ((InFileName == NULL) || (OutFileName == NULL)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_FILE_FIND_FAIL), gShellDebug1HiiHandle, L"efidecompress", TempParam);
ShellStatus = SHELL_NOT_FOUND;
} else {
@ -112,13 +118,25 @@ ShellCommandRunEfiDecompress (
if (ShellStatus == SHELL_SUCCESS) {
Status = FileHandleGetSize (InFileHandle, &Temp64Bit);
ASSERT_EFI_ERROR (Status);
if (!EFI_ERROR (Status)) {
ASSERT (Temp64Bit <= (UINT32)(-1));
InSize = (UINTN)Temp64Bit;
InBuffer = AllocateZeroPool (InSize);
if (EFI_ERROR (Status)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_FILE_OPEN_FAIL), gShellDebug1HiiHandle, L"efidecompress", ShellCommandLineGetRawValue (Package, 1));
ShellStatus = SHELL_NOT_FOUND;
}
}
if (ShellStatus == SHELL_SUCCESS) {
//
// Limit the File Size to UINT32, even though calls accept UINTN.
// 32 bits = 4gb.
//
Status = SafeUint64ToUint32 (Temp64Bit, (UINT32 *)&InSize);
if (EFI_ERROR (Status)) {
ASSERT_EFI_ERROR (Status);
ShellStatus = SHELL_BAD_BUFFER_SIZE;
goto Done;
}
InBuffer = AllocateZeroPool (InSize);
if (InBuffer == NULL) {
Status = EFI_OUT_OF_RESOURCES;
} else {
@ -166,6 +184,8 @@ ShellCommandRunEfiDecompress (
}
}
Done:
ShellCommandLineFreeVarList (Package);
}

View File

@ -371,6 +371,10 @@ HFileImageSave (
// set status string
//
Str = CatSPrint (NULL, L"%d Lines Written", NumLines);
if (Str == NULL) {
return EFI_OUT_OF_RESOURCES;
}
StatusBarSetStatusString (Str);
FreePool (Str);

View File

@ -115,7 +115,9 @@ HMainCommandDisplayHelp (
,
NULL
);
ShellPrintEx (0, CurrentLine+1, L"%E%s%N", InfoString);
if (InfoString != NULL) {
ShellPrintEx (0, CurrentLine+1, L"%E%s%N", InfoString);
}
}
//

View File

@ -302,6 +302,11 @@ LoadEfiDriversFromRomImage (
);
if (!EFI_ERROR (Status)) {
DecompressedImageBuffer = AllocateZeroPool (DestinationSize);
if (DecompressedImageBuffer == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"loadpcirom");
return EFI_OUT_OF_RESOURCES;
}
if (ImageBuffer != NULL) {
Scratch = AllocateZeroPool (ScratchSize);
if (Scratch != NULL) {
@ -333,6 +338,10 @@ LoadEfiDriversFromRomImage (
//
UnicodeSPrint (RomFileName, sizeof (RomFileName), L"%s[%d]", FileName, ImageIndex);
FilePath = FileDevicePath (NULL, RomFileName);
if (FilePath == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_LOADPCIROM_LOAD_FAIL), gShellDebug1HiiHandle, L"loadpcirom", FileName, ImageIndex);
return EFI_OUT_OF_RESOURCES;
}
Status = gBS->LoadImage (
TRUE,

View File

@ -214,7 +214,12 @@ ShellCommandRunMemMap (
if (Status == EFI_BUFFER_TOO_SMALL) {
Size += SIZE_1KB;
Descriptors = AllocateZeroPool (Size);
Status = gBS->GetMemoryMap (&Size, Descriptors, &MapKey, &ItemSize, &Version);
if (Descriptors == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"memmap");
ShellStatus = SHELL_OUT_OF_RESOURCES;
}
Status = gBS->GetMemoryMap (&Size, Descriptors, &MapKey, &ItemSize, &Version);
}
if (EFI_ERROR (Status)) {

View File

@ -537,7 +537,13 @@ ShellCommandRunMm (
goto Done;
}
Temp = ShellCommandLineGetRawValue (Package, 1);
Temp = ShellCommandLineGetRawValue (Package, 1);
if (Temp == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PROBLEM), gShellDebug1HiiHandle, L"mm", L"NULL");
ShellStatus = SHELL_INVALID_PARAMETER;
goto Done;
}
Status = ShellConvertStringToUint64 (Temp, &Address, TRUE, FALSE);
if (EFI_ERROR (Status)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"mm", Temp);

View File

@ -67,14 +67,20 @@ ShellCommandRunMode (
ShellStatus = SHELL_INVALID_PARAMETER;
} else if (ShellCommandLineGetCount (Package) == 3) {
Temp = ShellCommandLineGetRawValue (Package, 1);
if (!ShellIsHexOrDecimalNumber (Temp, FALSE, FALSE)) {
if (Temp == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"mode", Temp);
ShellStatus = SHELL_INVALID_PARAMETER;
} else if (!ShellIsHexOrDecimalNumber (Temp, FALSE, FALSE)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"mode", Temp);
ShellStatus = SHELL_INVALID_PARAMETER;
}
NewCol = ShellStrToUintn (Temp);
Temp = ShellCommandLineGetRawValue (Package, 2);
if (!ShellIsHexOrDecimalNumber (Temp, FALSE, FALSE)) {
if (Temp == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"mode", Temp);
ShellStatus = SHELL_INVALID_PARAMETER;
} else if (!ShellIsHexOrDecimalNumber (Temp, FALSE, FALSE)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"mode", Temp);
ShellStatus = SHELL_INVALID_PARAMETER;
}

View File

@ -5765,7 +5765,7 @@ PrintInterpretedExtendedCompatibilityDynamicPowerAllocation (
)
{
CONST PCI_EXPRESS_EXTENDED_CAPABILITIES_DYNAMIC_POWER_ALLOCATION *Header;
UINT8 LinkCount;
UINT32 LinkCount;
Header = (PCI_EXPRESS_EXTENDED_CAPABILITIES_DYNAMIC_POWER_ALLOCATION *)HeaderAddress;
@ -5780,7 +5780,7 @@ PrintInterpretedExtendedCompatibilityDynamicPowerAllocation (
Header->DpaStatus,
Header->DpaControl
);
for (LinkCount = 0; LinkCount < PCI_EXPRESS_EXTENDED_CAPABILITY_DYNAMIC_POWER_ALLOCATION_GET_SUBSTATE_MAX (Header) + 1; LinkCount++) {
for (LinkCount = 0; LinkCount < PCI_EXPRESS_EXTENDED_CAPABILITY_DYNAMIC_POWER_ALLOCATION_GET_SUBSTATE_MAX (Header) + (UINT32)1; LinkCount++) {
ShellPrintHiiEx (
-1,
-1,

View File

@ -288,7 +288,13 @@ ShellCommandRunSerMode (
goto Done;
}
Temp = ShellCommandLineGetRawValue (Package, 5);
Temp = ShellCommandLineGetRawValue (Package, 5);
if (Temp == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"sermode");
ShellStatus = SHELL_INVALID_PARAMETER;
goto Done;
}
Value = ShellStrToUintn (Temp);
switch (Value) {
case 0:

View File

@ -62,7 +62,11 @@ ShellCommandRunSetSize (
NewSize = 0;
} else {
Temp1 = ShellCommandLineGetRawValue (Package, 1);
if (!ShellIsHexOrDecimalNumber (Temp1, FALSE, FALSE)) {
if (Temp1 == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_FEW), gShellDebug1HiiHandle, L"setsize");
ShellStatus = SHELL_INVALID_PARAMETER;
NewSize = 0;
} else if (!ShellIsHexOrDecimalNumber (Temp1, FALSE, FALSE)) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_SIZE_NOT_SPEC), gShellDebug1HiiHandle, L"setsize");
ShellStatus = SHELL_INVALID_PARAMETER;
NewSize = 0;

View File

@ -285,7 +285,10 @@ GetVariableDataFromParameter (
for (Index = 2; Index < ShellCommandLineGetCount (Package); Index++) {
TempData = ShellCommandLineGetRawValue (Package, Index);
ASSERT (TempData != NULL);
if (TempData == NULL) {
ASSERT (TempData != NULL);
return EFI_INVALID_PARAMETER;
}
if (TempData[0] != L'=') {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"setvar", TempData);
@ -401,11 +404,21 @@ ShellCommandRunSetVar (
ShellStatus = SHELL_INVALID_PARAMETER;
} else {
VariableName = ShellCommandLineGetRawValue (Package, 1);
if (VariableName == NULL) {
return SHELL_INVALID_PARAMETER;
}
if (!ShellCommandLineGetFlag (Package, L"-guid")) {
CopyGuid (&Guid, &gEfiGlobalVariableGuid);
} else {
StringGuid = ShellCommandLineGetValue (Package, L"-guid");
RStatus = StrToGuid (StringGuid, &Guid);
if (StringGuid != NULL) {
RStatus = StrToGuid (StringGuid, &Guid);
} else {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"setvar", StringGuid);
return SHELL_INVALID_PARAMETER;
}
if (RETURN_ERROR (RStatus) || (StringGuid[GUID_STRING_LENGTH] != L'\0')) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), gShellDebug1HiiHandle, L"setvar", StringGuid);
ShellStatus = SHELL_INVALID_PARAMETER;
@ -419,6 +432,11 @@ ShellCommandRunSetVar (
Status = gRT->GetVariable ((CHAR16 *)VariableName, &Guid, &Attributes, &Size, Buffer);
if (Status == EFI_BUFFER_TOO_SMALL) {
Buffer = AllocateZeroPool (Size);
if (Buffer == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"setvar");
return SHELL_OUT_OF_RESOURCES;
}
Status = gRT->GetVariable ((CHAR16 *)VariableName, &Guid, &Attributes, &Size, Buffer);
}
@ -440,6 +458,11 @@ ShellCommandRunSetVar (
Status = gRT->GetVariable ((CHAR16 *)VariableName, &Guid, &Attributes, &Size, Buffer);
if (Status == EFI_BUFFER_TOO_SMALL) {
Buffer = AllocateZeroPool (Size);
if (Buffer == NULL) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"setvar");
return SHELL_OUT_OF_RESOURCES;
}
Status = gRT->GetVariable ((CHAR16 *)VariableName, &Guid, &Attributes, &Size, Buffer);
}

View File

@ -676,7 +676,7 @@ SmbiosPrintStructure (
{
UINTN NumOfDevice;
NumOfDevice = (Struct->Type10->Hdr.Length - sizeof (SMBIOS_STRUCTURE)) / (2 * sizeof (UINT8));
for (Index = 0; Index < NumOfDevice; Index++) {
for (Index = 0; (UINTN)Index < NumOfDevice; Index++) {
ShellPrintEx (-1, -1, (((Struct->Type10->Device[Index].DeviceType) & 0x80) != 0) ? L"Device Enabled\n" : L"Device Disabled\n");
DisplayOnboardDeviceTypes ((Struct->Type10->Device[Index].DeviceType) & 0x7F, Option);
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_DESC_STRING), gShellDebug1HiiHandle);

View File

@ -790,7 +790,7 @@ InitSmbios64BitTableStatistics (
//
Handle = INVALID_HANDLE;
LibGetSmbios64BitStructure (&Handle, NULL, NULL);
for (Index = 1; Index <= mNumberOfSmbios64BitStructures; Index++) {
for (Index = 1; (UINTN)Index <= mNumberOfSmbios64BitStructures; Index++) {
//
// If reach the end of table, break..
//

View File

@ -269,6 +269,10 @@ EditGetDefaultFileName (
do {
FileNameTmp = CatSPrint (NULL, L"NewFile%d.%s", Suffix, Extension);
if (FileNameTmp == NULL) {
ASSERT (FileNameTmp != NULL);
return NULL;
}
//
// after that filename changed to path

View File

@ -50,6 +50,7 @@
#include <Library/DevicePathLib.h>
#include <Library/PrintLib.h>
#include <Library/HandleParsingLib.h>
#include <Library/SafeIntLib.h>
extern EFI_HII_HANDLE gShellDebug1HiiHandle;

View File

@ -111,6 +111,7 @@
SortLib
PrintLib
BcfgCommandLib
SafeIntLib
[Pcd]
gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize ## CONSUMES

View File

@ -65,6 +65,8 @@
DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
# StackCheckLib is not linked for SEC modules by default, this package can link it against its SEC modules
[LibraryClasses.common.SEC]
NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf