uefi-ref/edk2
1
0
Fork 0
mirror of https://github.com/tianocore/edk2 synced 2026-08-27 00:23:19 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

ShellPkg/Shell: Terminate and separate the ShellOpt command line

Browse source 
CreatePopulateInstallShellParametersProtocol() built the command line
from ShellOpt and image load options. The buffer was sized to exactly
Size + LoadOptionsSize bytes. Neither source needs to be
NUL-terminated, so ParseCommandLineToArgs() could read beyond the
allocation and treat unrelated pool memory as command line arguments.

When both sources were present, the image load options were copied at
offset zero. This overwrote ShellOpt instead of following it.

Allocate a separator and terminator. Reject odd-length ShellOpt data.
Remove its trailing terminator when present. Append load options after
ShellOpt, adding a separator only when both contain data.
This matches the order documented in the shell help text:

  shell.efi [ShellOpt-options] [options] [file-name [options]]

Fixes: https://github.com/tianocore/edk2/issues/10549
Signed-off-by: Jin Kang <112378607+kimkimkimk@users.noreply.github.com>
This commit is contained in:
Jin Kang 2026-08-24 18:52:54 +08:00
parent 7735ed4f8e
commit 18a487908f
1 changed files with 66 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

74
ShellPkg/Application/Shell/ShellParametersProtocol.c
View file

@ -353,35 +353,93 @@ CreatePopulateInstallShellParametersProtocol (
//
Status = SHELL_GET_ENVIRONMENT_VARIABLE (L"ShellOpt", &Size, FullCommandLine);
if (Status == EFI_BUFFER_TOO_SMALL) {
FullCommandLine = AllocateZeroPool (Size + LoadedImage->LoadOptionsSize);
//
// The ShellOpt options come first, followed by the image load options.
// Variable data and load options are not guaranteed to be NUL-terminated,
// and the two parts must be separated when they are joined, so allocate
// room for a space and a NUL terminator in addition to both parts.
//
FullCommandLine = AllocateZeroPool (
Size + LoadedImage->LoadOptionsSize +
2 * sizeof (FullCommandLine[0])
);
if (FullCommandLine == NULL) {
return EFI_OUT_OF_RESOURCES;
}
Status = SHELL_GET_ENVIRONMENT_VARIABLE (L"ShellOpt", &Size, FullCommandLine);
if (EFI_ERROR (Status)) {
FreePool (FullCommandLine);
FullCommandLine = NULL;
Size = 0;
} else {
//
// ShellOpt is a CHAR16 string. Reject malformed variable data before
// indexing the buffer in CHAR16 units.
//
if ((Size % sizeof (CHAR16)) != 0) {
FreePool (FullCommandLine);
return EFI_INVALID_PARAMETER;
}
//
// Drop the variable's own terminator, if any; a fresh terminator lives
// in the zero-initialized tail of the allocation.
//
if ((Size >= sizeof (CHAR16)) &&
((Size % sizeof (CHAR16)) == 0) &&
(FullCommandLine[Size / sizeof (CHAR16) - 1] == CHAR_NULL))
{
Size -= sizeof (CHAR16);
}
if (LoadedImage->LoadOptionsSize != 0) {
if (Size != 0) {
FullCommandLine[Size / sizeof (CHAR16)] = L' ';
Size += sizeof (CHAR16);
}
CopyMem (
(UINT8 *)FullCommandLine + Size,
LoadedImage->LoadOptions,
LoadedImage->LoadOptionsSize
);
}
}
}
if (Status == EFI_NOT_FOUND) {
//
// no parameters via environment... ok
// No parameters via environment... ok.
//
} else {
if (EFI_ERROR (Status)) {
return (Status);
return Status;
}
}
if ((Size == 0) && (LoadedImage->LoadOptionsSize != 0)) {
ASSERT (FullCommandLine == NULL);
if ((FullCommandLine == NULL) &&
(LoadedImage->LoadOptionsSize != 0))
{
//
// Now we need to include a NULL terminator in the size.
// ShellOpt is absent or is a zero-length variable. Allocate a
// command line containing only the image load options.
//
Size = LoadedImage->LoadOptionsSize + sizeof (FullCommandLine[0]);
Size = LoadedImage->LoadOptionsSize +
sizeof (FullCommandLine[0]);
FullCommandLine = AllocateZeroPool (Size);
if (FullCommandLine == NULL) {
return EFI_OUT_OF_RESOURCES;
}
CopyMem (
FullCommandLine,
LoadedImage->LoadOptions,
LoadedImage->LoadOptionsSize
);
}
if (FullCommandLine != NULL) {
CopyMem (FullCommandLine, LoadedImage->LoadOptions, LoadedImage->LoadOptionsSize);
//
// Populate Argc and Argv
//