PowerShellMafia / PowerSploit

PowerSploit - A PowerShell Post-Exploitation Framework
Other
11.96k stars 4.62k forks source link

PowerUp Get-UnquotedService doesn't check correct paths #248

Closed Raikia closed 7 years ago

Raikia commented 7 years ago

Specifically this line: https://github.com/PowerShellMafia/PowerSploit/blob/dev/Privesc/PowerUp.ps1#L2072

$ModifiableFiles = $Service.pathname.Split(' ') | Get-ModifiablePath

For a path like "C:\Program Files\VMware Workstation\test.exe":

Get-ModifiablePath expects: @("C:\Program", "C\Program Files\VMware", "C:\Program Files\VMware Workstation\test.exe")

However, it receives: @("C:\Program", "Files\VMware", "Workstation\test.exe"). Get-ModifiablePath checks to see if each of those directories is writable, which returns nothing for "Files\VMware" and "Workstation\test.exe" because it doesn't obviously exist without the prefix of "C:\Program".

Screenshot proof: http://i.imgur.com/8thoDIe.png

Better code drop-in replacement (but someone better at PowerShell than me can probably do better since I have 0 PowerShell background aside from stumbling through):

$SplitPath = $Service.pathname.split(' ')
$PossiblePaths = @()
$PathStart = ""
ForEach ($chunk in $SplitPath) {
      if ($PathStart -ne "") {
         $PathStart += " "
      }
     $PathStart  += $chunk
     $PossiblePaths += $PathStart
}
$ModifiableFiles = $PossiblePaths | Get-ModifiablePath

It looks like this issue affects both master and dev branches.

FuzzySecurity commented 7 years ago

Mm, maybe something like this:

$InputPath = "C:\Program Files\VMware Workstation\test.exe"
$SplitPathArray = $InputPath.Split(' ')
$ResultArray = @()
for ($i=0;$i -lt $SplitPathArray.Count; $i++) {
    $ResultArray += $SplitPathArray[0..$i] -join ' '
}
--------------------------------------------------------
PS C:\Users\b33f> $ResultArray
C:\Program
C:\Program Files\VMware
C:\Program Files\VMware Workstation\test.exe
FuzzySecurity commented 7 years ago

As a patch this seems to work fine:

    if ($VulnServices) {
        ForEach ($Service in $VulnServices) {

            $SplitPathArray = $Service.pathname.Split(' ')
            $ConcatPathArray = @()
            for ($i=0;$i -lt $SplitPathArray.Count; $i++) {
                        $ConcatPathArray += $SplitPathArray[0..$i] -join ' '
            }

            $ModifiableFiles = $ConcatPathArray | Get-ModifiablePath

            $ModifiableFiles | Where-Object {$_ -and $_.ModifiablePath -and ($_.ModifiablePath -ne '')} | Foreach-Object {
                $CanRestart = Test-ServiceDaclPermission -PermissionSet 'Restart' -Name $Service.name
                $Out = New-Object PSObject
                $Out | Add-Member Noteproperty 'ServiceName' $Service.name
                $Out | Add-Member Noteproperty 'Path' $Service.pathname
                $Out | Add-Member Noteproperty 'ModifiablePath' $_
                $Out | Add-Member Noteproperty 'StartName' $Service.startname
                $Out | Add-Member Noteproperty 'AbuseFunction' "Write-ServiceBinary -Name '$($Service.name)' -Path <HijackPath>"
                $Out | Add-Member Noteproperty 'CanRestart' ([Bool]$CanRestart)
                $Out.PSObject.TypeNames.Insert(0, 'PowerUp.UnquotedService')
                $Out
            }
        }
    }

I did not push to Dev though because I'm unsure about the behavior of this function: image

It seems to me it should only report:

Maybe @HarmJ0y has some ideas?

FuzzySecurity commented 7 years ago

Ok, so on closer review I think this function is kind of working as expected and I pushed a fix here https://github.com/PowerShellMafia/PowerSploit/commit/3d0d32d9ee6af70f0dfd5ecfe809a49a65d6822d

The script won't return a path like:

C:\Python27\Test

This is because Get-ModifiablePath will check the parent folder if the path is not valid, which it isn't in this case. The other point is that the function appears to return duplicate entries. Also not a bug, I think, because each duplicate path entry shows distinct permission sets. See image below.

image

It may be a good idea to consolidate the permissions but that could break functionality elsewhere in Powerup. Closing this for now.