Invoke-Expression: testing for all potential code injection possibilities-CodePudding

I am looking to define some complex conditional statements in XML, since I now have 20 different conditions I need to test, and I could well need to add more, and I would rather add those conditions by providing an updated XML file rather than issuing revised code. For what it's worth, I am trying to identify correct Autodesk Uninstall strings because Autodesk doesn't follow Windows standards at all. Their new installers will have their own special uninstall that uses their ODIS executable, while also having a bunch of standard MSIExec based uninstalls that do nothing and/or cause errors. I am trying to parse all the Uninstalls and filter out just the ones that work right. And they are VERY inconsistent, thus 20 different conditions at this point. So what I have is a $rawKeys variable that contains all the keys from the Uninstall key in the registry, and I loop through all of them and do some comparisons and adding anything that meets the skip criteria to a $skip variable so when I encounter that later in the loop I can just skip over it. So... An example of a conditional that identifies a particular Autodesk Uninstall that has invalid uninstalls keys as well looks like this.

if ($key.guid -and ($key.publisher -like 'Autodesk*') -and ($key.uninstallString -like '*AdODIS*') -and
   ($skip = $rawKeys.Where({$_.guid -and ($_.publisher -like 'Autodesk*') -and ($_.displayName -eq $key.displayName) -and ( $_.uninstallString)}))) {

In moving that data to XML I have

<Pattern>
        <Find>
            <Criteria property="$key.guid" operator="-eq" value="exists" />
            <Criteria property="$key.publisher" operator="-like" value="Autodesk*" />
            <Criteria property="$key.uninstallString" operator="-like" value="*AdODIS*" />
        </Find>
        <Skip>
            <Criteria property="$PSItem.guid" operator="-eq" value="exists" />
            <Criteria property="$PSItem.publisher" operator="-like" value="Autodesk*" />
            <Criteria property="$PSItem.displayName" operator="-eq" value="$key.displayName" />
            <Criteria property="$PSItem.uninstallString" operator="-eq" value="exists" />
        </Skip>
    </Pattern>

And I have verified that I can use Invoke-Expression to extract correct data from the variable/Xml. The validation there is simply

$hash = @{
    one = '1'
    two = '2'
}
$string = '$hash.two'
Invoke-Expression $string

Which correctly produces 2

But of course Invoke-Expression is at risk of malicious code injection, which can be seen with

$string = '$hash.two; Write-Host'

I have done some searching and every post I find specifically mentions injecting code using the command separator ;. And my good data will never have a semi-colon, so a first line of defense would seem to be simply to look for a semi colon and if found throw an exception. But a malicious actor could also simply replace the entire value like this

$string = 'Write-Host "Malicious Code"'

Given that my good values will never contain a space, I could search for space and thus catch any Cmdlet with arguments. And finally, the malicious code could be just a Cmdlet, so I could use

Get-Command $string

and only Invoke-Expression on a failure.

I THINK that covers all bases, but I have yet to find someone who offers a definitive "Safe parsing of strings before Invoke-Expression" example, and I would really like to be more certain that I really have covered all bases. That, or be convinced that this is such a bad idea that I should simply keep all the conditionals in code and provide updates in new, signed PSM1s.

Another option I am looking at is to sign the XML and test that at XML load. If the XML has been changed in any way I throw an error. I suspect that is an additional layer of security, not a replacement for the in code validations mentioned above.

EDIT: Sigh. It seems I do have a scenario with a space.

$_.uninstallString -eq "MsiExec.exe /X$($_.guid)")

But, if I wrap the variables in $() as shown here, in the XML, then

$ExecutionContext.InvokeCommand.ExpandString($string)

will expand things appropriately. And from there, since I will never use the expanded data for execution, only for string comparisons, I am safe. I THINK. :)

CodePudding user response：

As you already noted, Invoke-Expression should only be used under controlled conditions, it should be avoided when the expression can be provided by users, as it is susceptible to script injection. The same can be said about $ExecutionContext.InvokeCommand.ExpandString(). The latter can be exploited by use of the subexpression $(…) operator, which can run arbitrary commands.

A safe way (to my knowledge) for preventing script injection through user-defined expressions, is the ScriptBlock.CheckRestrictedLanguage() method.

Here is a simplified example:

# Variables to be used in the scriptblock
$a = 23; $b = 42

# Create a scriptblock, which could be user input, which is not executed yet.
$sb = [scriptblock]::Create('$a -lt $b')

# Define restrictions for the scriptblock
$allowedCommands  = [string[]] @()          # no allowed commands
$allowedVariables = [string[]] @('a', 'b')  # only these variables can be used
$allowEnvVars     = $false                  # environment variables not allowed

# Check the scriptblock against the restrictions
# This throws an exception if the scriptblock doesn't comply to the restrictions.
$sb.CheckRestrictedLanguage( $allowedCommands, $allowedVariables, $allowEnvVars )

# If we reach here, executing the scriptblock is safe.
& $sb

The above code runs successfully, as the expression $a -lt $b only uses the allowed variables and doesn't call any command.

Here are some examples that would cause the script to terminate with an error, because of the restrictions we have defined:

$a -lt $x         # invalid variable x - only a and b are allowed
$env:SystemRoot   # environment variables are not allowed
Write-Host 42     # commands are not allowed
.\SomeScript.ps1  # commands are not allowed