diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c b/ShellPkg/Application/Shell/ShellParametersProtocol.c index ece7a33f93..0e2a62cf56 100644 --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c @@ -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 //