A PowerShell function needs to run a .exe whose location is not known at design-time because it's part of a NuGet package. If I run it using Invoke-Expression
then PSScriptAnalyzer raises a warning telling me to find an alternative method, however if I run it using &
then I get no warning.
This is the function, which is part of a module for running unit tests and generating a code coverage report on a machine where the installed IDE does not support code coverage (e.g. Visual Studio Community Edition).
function Invoke-ReportGenerator {
param (
[Parameter(Mandatory=$true)][string]$testProjectFolder,
[Parameter(Mandatory=$true)][string]$testProjectName,
[Parameter(Mandatory=$true)][string]$assemblyUnderTest,
[Parameter(Mandatory=$false)][string]$coverageXmlFilename,
[Parameter(Mandatory=$false)][string]$reportGeneratorPath
)
if ([System.String]::IsNullOrWhiteSpace($coverageXmlFilename)) {
$coverageXmlFilename = "coverage.opencover.xml";
}
$absoluteOutputPath = [System.IO.Path]::Combine($testProjectFolder, "CodeCoverage");
$absoluteInputPath = [System.IO.Path]::Combine($testProjectFolder, $coverageXmlFilename);
$argumentArray = @(
"-reports:$absoluteInputPath",
"-targetDir:$absoluteOutputPath",
"-title:$testProjectName",
"-assemblyFilters:$assemblyUnderTest"
);
if ([System.String]::IsNullOrWhiteSpace($reportGeneratorPath)) {
$reportGeneratorPath = "$env:USERPROFILE\.nuget\packages\reportgenerator\5.2.4\tools\net6.0\reportgenerator.exe ";
}
$reportGeneratorCommand = "$reportGeneratorPath $argumentArray";
Invoke-Expression $reportGeneratorCommand;
}
This is the warning raised by PSScriptAnalyzer
Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead.
Microsoft docs say to Avoid using Invoke-Expression
Carefully consider the security implications. When a string from an untrusted source such as user input is passed directly to Invoke-Expression, arbitrary commands can be executed. Always consider a different, more robust and secure solution first.
If I change the line
Invoke-Expression $reportGeneratorCommand;
to
& $reportGeneratorCommand;
Then the warning goes away. But surely this is just as vulnerable to a code injection attack as Invoke-Expression
? What am I missing?
Edit: replacing the last 2 lines of the function with & $reportGeneratorPath @argumentArray
as suggested by @MathiasRJessen simplifies the function slightly, but I believe still leaves it vulnerable.
The difference between Invoke-Expression
and the invocation operators (&
and .
) is structured syntax.
Unlike Invoke-Expression
, the operators accept the target command and the arguments separately, using the following syntax:
<op> [<module-spec>] <target-command> [arguments...]
... and as suggested in the comments, you should use them in conjunction with splatting, like this:
& $command @arguments
but I believe still leaves it vulnerable.
Perhaps the following example will shake your beliefs.
Let's start by defining a target command and some user-suplied arguments including an attempted command injection:
$command = 'Write-Host'
$arguments = @(
'Legitimate message'
';'
'Write-Host'
'malware'
)
Let's see what happens when we construct a string and pass it off to Invoke-Expression
like you've been doing so far:
PS ~> $expression = "$command $arguments"
PS ~> Invoke-Expression $expression
Legitimate message
malware
As you can see, Invoke-Expression
happily evaluated both the target command and the Write-Host malware
command tacked at the end.
Let's try the same with an invocation operator:
PS ~> & $command @arguments
Legitimate message ; Write-Host malware
As you see here, all arguments were passed as just that - no invocation of any additional commands injected by the caller.