powershellcode-injection

Is the call (&) operator less vulnerable to code injection attacks than Invoke-Expression?


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.


Solution

  • 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.