Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(#3424) Fix potential recursive elevation attempts #3425

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

vexx32
Copy link
Member

@vexx32 vexx32 commented Apr 19, 2024

Description Of Changes

Add a guard into Start-ChocolateyProcessAsAdmin to ensure we don't accidentally recurse in the case of calling back into helper commands that themselves call Start-ChocolateyProcessAsAdmin.

I don't like how messy the error is, but it's better than spawning infinite processes, and I'm not clear on how to tidy up the error in this case as the clixml is not correctly consumed by the receiving PowerShell process.

Motivation and Context

This shouldn't recurse like this, if the first elevation fails, retrying it is silly.

Testing

I'd like to add tests for this, but I think all our testing currently runs in an admin context, so there's... not much we can do there.

  1. Download the following test package to the C:\packages folder: add-path.1.0.0.zip (rename to .nupkg)
  2. On a relatively clean VM you don't mind stressing out a bit, copy a normal Chocolatey install to your user folder.
  3. Open a PowerShell session to the copied Chocolatey folder.
  4. Set $env:ChocolateyInstall = Get-Location
  5. Run ./choco.exe install add-path -s c:\packages -y --params="'/Path=c:\tools /Scope=Machine'"

On current choco versions, this test will break spectacularly, generating endless powershell.exe instances until the processes are killed.

With this change, we instead get a messy, but reasonable, error that the elevation failed:

#< CLIXML
<Objs Version="1.1.0.1" xmlns="http://schemas.microsoft.com/powershell/2004/04"><Obj S="progress" RefId="0"><TN RefId="0"><T>System.Management.Automation.PSCustomObject</T><T>System.Object</T></TN><MS><I64 N="SourceId">1</I64><PR N="Record"><AV>Preparing modules for first use.</AV><AI>0</AI><Nil /><PI>-1</PI><PC>-1</PC><T>Completed</T><SR>-1</SR><SD> </SD></PR></MS></Obj><Obj S="progress" RefId="1"><TNRef RefId="0" /><MS><I64 N="SourceId">1</I64><PR N="Record"><AV>Preparing modules for first use.</AV><AI>0</AI><Nil /><PI>-1</PI><PC>-1</PC><T>Completed</T><SR>-1</SR><SD> </SD></PR></MS></Obj><S S="debug">Host version is 5.1.14393.2791, PowerShell Version is '5.1.14393.2791' and CLR Version is '4.0.30319.42000'.</S><S S="verbose">Exporting function 'Format-FileSize'.</S><S S="verbose">Exporting function 'Get-ChecksumValid'.</S><S S="verbose">Exporting function 'Get-ChocolateyConfigValue'.</S><S S="verbose">Exporting function 'Get-ChocolateyPath'.</S><S S="verbose">Exporting function 'Get-ChocolateyUnzip'.</S><S S="verbose">Exporting function 'Get-ChocolateyWebFile'.</S><S S="verbose">Exporting function 'Get-EnvironmentVariable'.</S><S S="verbose">Exporting function 'Get-EnvironmentVariableNames'.</S><S S="verbose">Exporting function 'Get-FtpFile'.</S><S S="verbose">Exporting function 'Get-OSArchitectureWidth'.</S><S S="verbose">Exporting function 'Get-PackageParameters'.</S><S S="verbose">Exporting function 'Get-PackageParametersBuiltIn'.</S><S S="verbose">Exporting function 'Get-ToolsLocation'.</S><S S="verbose">Exporting function 'Get-UACEnabled'.</S><S S="verbose">Exporting function 'Get-UninstallRegistryKey'.</S><S S="verbose">Exporting function 'Get-VirusCheckValid'.</S><S S="verbose">Exporting function 'Get-WebFile'.</S><S S="verbose">Exporting function 'Get-WebFileName'.</S><S S="verbose">Exporting function 'Get-WebHeaders'.</S><S S="verbose">Exporting function 'Install-BinFile'.</S><S S="verbose">Exporting function 'Install-ChocolateyEnvironmentVariable'.</S><S S="verbose">Exporting function 'Install-ChocolateyExplorerMenuItem'.</S><S S="verbose">Exporting function 'Install-ChocolateyFileAssociation'.</S><S S="verbose">Exporting function 'Install-ChocolateyInstallPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyPath'.</S><S S="verbose">Exporting function 'Install-ChocolateyPinnedTaskBarItem'.</S><S S="verbose">Exporting function 'Install-ChocolateyPowershellCommand'.</S><S S="verbose">Exporting function 'Install-ChocolateyShortcut'.</S><S S="verbose">Exporting function 'Install-ChocolateyVsixPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyZipPackage'.</S><S S="verbose">Exporting function 'Install-Vsix'.</S><S S="verbose">Exporting function 'Set-EnvironmentVariable'.</S><S S="verbose">Exporting function 'Set-PowerShellExitCode'.</S><S S="verbose">Exporting function 'Start-ChocolateyProcessAsAdmin'.</S><S S="verbose">Exporting function 'Test-ProcessAdminRights'.</S><S S="verbose">Exporting function 'Uninstall-BinFile'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyEnvironmentVariable'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyPackage'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyZipPackage'.</S><S S="verbose">Exporting function 'Update-SessionEnvironment'.</S><S S="verbose">Exporting function 'Write-FunctionCallLogMessage'.</S><S S="verbose">Exporting alias 'Get-ProcessorBits'.</S><S S="verbose">Exporting alias 'Get-OSBitness'.</S><S S="verbose">Exporting alias 'Get-InstallRegistryKey'.</S><S S="verbose">Exporting alias 'Generate-BinFile'.</S><S S="verbose">Exporting alias 'Add-BinFile'.</S><S S="verbose">Exporting alias 'Start-ChocolateyProcess'.</S><S S="verbose">Exporting alias 'Invoke-ChocolateyProcess'.</S><S S="verbose">Exporting alias 'Remove-BinFile'.</S><S S="verbose">Exporting alias 'refreshenv'.</S><S S="debug">Importing 'C:\Users\vagrant\chocolatey\helpers\..\extensions\chocolatey\chocolatey.licensed.dll'</S><S S="debug">Loading 'chocolatey.licensed' extension</S><S S="verbose">Loading module from path 'C:\Users\vagrant\chocolatey\extensions\chocolatey\chocolatey.licensed.dll'.</S><S S="warning">Import failed for Chocolatey Licensed Extension. Error: 'Could not load file or assembly 'chocolatey, Version=2.1.0.0, Culture=neutral, PublicKeyToken=79d02ea9cad655eb' or one of its dependencies. The system cannot find the file specified.'</S><S S="debug">Loading community extensions</S><S S="debug">Importing 'C:\Users\vagrant\chocolatey\extensions\chocolatey\chocolatey.extensions.psm1'</S><S S="verbose">Loading module from path 'C:\Users\vagrant\chocolatey\extensions\chocolatey\chocolatey.extensions.psm1'.</S><S S="debug">Importing 'C:\Users\vagrant\chocolatey\extensions\chocolatey-compatibility\chocolatey-compatibility.psm1'</S><S S="verbose">Loading module from path 'C:\Users\vagrant\chocolatey\extensions\chocolatey-compatibility\chocolatey-compatibility.psm1'.</S><S S="debug">Function 'Get-PackageParameters' exists, ignoring export.</S><S S="debug">Function 'Get-UninstallRegistryKey' exists, ignoring export.</S><S S="debug">Exporting function 'Install-ChocolateyDesktopLink' for backwards compatibility</S><S S="verbose">Exporting function 'Install-ChocolateyDesktopLink'.</S><S S="debug">Exporting function 'Write-ChocolateyFailure' for backwards compatibility</S><S S="verbose">Exporting function 'Write-ChocolateyFailure'.</S><S S="debug">Exporting function 'Write-ChocolateySuccess' for backwards compatibility</S><S S="verbose">Exporting function 'Write-ChocolateySuccess'.</S><S S="debug">Exporting function 'Write-FileUpdateLog' for backwards compatibility</S><S S="verbose">Exporting function 'Write-FileUpdateLog'.</S><S S="verbose">Importing function 'Install-ChocolateyDesktopLink'.</S><S S="verbose">Importing function 'Write-ChocolateyFailure'.</S><S S="verbose">Importing function 'Write-ChocolateySuccess'.</S><S S="verbose">Importing function 'Write-FileUpdateLog'.</S><S S="debug">Importing 'C:\Users\vagrant\chocolatey\extensions\chocolatey-core\chocolatey-core.psm1'</S><S S="verbose">Loading module from path 'C:\Users\vagrant\chocolatey\extensions\chocolatey-core\chocolatey-core.psm1'.</S><S S="verbose">Exporting function 'Get-AppInstallLocation'.</S><S S="verbose">Exporting function 'Get-AvailableDriveLetter'.</S><S S="verbose">Exporting function 'Get-EffectiveProxy'.</S><S S="verbose">Exporting function 'Get-PackageCacheLocation'.</S><S S="verbose">Exporting function 'Get-WebContent'.</S><S S="verbose">Exporting function 'Register-Application'.</S><S S="verbose">Exporting function 'Remove-Process'.</S><S S="verbose">Importing function 'Get-AppInstallLocation'.</S><S S="verbose">Importing function 'Get-AvailableDriveLetter'.</S><S S="verbose">Importing function 'Get-EffectiveProxy'.</S><S S="verbose">Importing function 'Get-PackageCacheLocation'.</S><S S="verbose">Importing function 'Get-WebContent'.</S><S S="verbose">Importing function 'Register-Application'.</S><S S="verbose">Importing function 'Remove-Process'.</S><S S="verbose">Exporting function 'Format-FileSize'.</S><S S="verbose">Exporting function 'Get-ChecksumValid'.</S><S S="verbose">Exporting function 'Get-ChocolateyConfigValue'.</S><S S="verbose">Exporting function 'Get-ChocolateyPath'.</S><S S="verbose">Exporting function 'Get-ChocolateyUnzip'.</S><S S="verbose">Exporting function 'Get-ChocolateyWebFile'.</S><S S="verbose">Exporting function 'Get-EnvironmentVariable'.</S><S S="verbose">Exporting function 'Get-EnvironmentVariableNames'.</S><S S="verbose">Exporting function 'Get-FtpFile'.</S><S S="verbose">Exporting function 'Get-OSArchitectureWidth'.</S><S S="verbose">Exporting function 'Get-PackageParameters'.</S><S S="verbose">Exporting function 'Get-PackageParametersBuiltIn'.</S><S S="verbose">Exporting function 'Get-ToolsLocation'.</S><S S="verbose">Exporting function 'Get-UACEnabled'.</S><S S="verbose">Exporting function 'Get-UninstallRegistryKey'.</S><S S="verbose">Exporting function 'Get-VirusCheckValid'.</S><S S="verbose">Exporting function 'Get-WebFile'.</S><S S="verbose">Exporting function 'Get-WebFileName'.</S><S S="verbose">Exporting function 'Get-WebHeaders'.</S><S S="verbose">Exporting function 'Install-BinFile'.</S><S S="verbose">Exporting function 'Install-ChocolateyEnvironmentVariable'.</S><S S="verbose">Exporting function 'Install-ChocolateyExplorerMenuItem'.</S><S S="verbose">Exporting function 'Install-ChocolateyFileAssociation'.</S><S S="verbose">Exporting function 'Install-ChocolateyInstallPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyPath'.</S><S S="verbose">Exporting function 'Install-ChocolateyPinnedTaskBarItem'.</S><S S="verbose">Exporting function 'Install-ChocolateyPowershellCommand'.</S><S S="verbose">Exporting function 'Install-ChocolateyShortcut'.</S><S S="verbose">Exporting function 'Install-ChocolateyVsixPackage'.</S><S S="verbose">Exporting function 'Install-ChocolateyZipPackage'.</S><S S="verbose">Exporting function 'Install-Vsix'.</S><S S="verbose">Exporting function 'Set-EnvironmentVariable'.</S><S S="verbose">Exporting function 'Set-PowerShellExitCode'.</S><S S="verbose">Exporting function 'Start-ChocolateyProcessAsAdmin'.</S><S S="verbose">Exporting function 'Test-ProcessAdminRights'.</S><S S="verbose">Exporting function 'Uninstall-BinFile'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyEnvironmentVariable'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyPackage'.</S><S S="verbose">Exporting function 'Uninstall-ChocolateyZipPackage'.</S><S S="verbose">Exporting function 'Update-SessionEnvironment'.</S><S S="verbose">Exporting function 'Write-FunctionCallLogMessage'.</S><S S="verbose">Exporting function 'Install-ChocolateyDesktopLink'.</S><S S="verbose">Exporting function 'Write-ChocolateyFailure'.</S><S S="verbose">Exporting function 'Write-ChocolateySuccess'.</S><S S="verbose">Exporting function 'Write-FileUpdateLog'.</S><S S="verbose">Exporting function 'Get-AppInstallLocation'.</S><S S="verbose">Exporting function 'Get-AvailableDriveLetter'.</S><S S="verbose">Exporting function 'Get-EffectiveProxy'.</S><S S="verbose">Exporting function 'Get-PackageCacheLocation'.</S><S S="verbose">Exporting function 'Get-WebContent'.</S><S S="verbose">Exporting function 'Register-Application'.</S><S S="verbose">Exporting function 'Remove-Process'.</S><S S="verbose">Exporting alias 'Get-ProcessorBits'.</S><S S="verbose">Exporting alias 'Get-OSBitness'.</S><S S="verbose">Exporting alias 'Get-InstallRegistryKey'.</S><S S="verbose">Exporting alias 'Generate-BinFile'.</S><S S="verbose">Exporting alias 'Add-BinFile'.</S><S S="verbose">Exporting alias 'Start-ChocolateyProcess'.</S><S S="verbose">Exporting alias 'Invoke-ChocolateyProcess'.</S><S S="verbose">Exporting alias 'Remove-BinFile'.</S><S S="verbose">Exporting alias 'refreshenv'.</S><S S="debug">Running Install-ChocolateyPath -pathToInstall 'C:\tools' -pathType 'Machine' </S><S S="debug">Running Update-SessionEnvironment </S><S S="verbose">Refreshing environment variables from the registry.</S><Obj S="information" RefId="2"><TN RefId="1"><T>System.Management.Automation.InformationRecord</T><T>System.Object</T></TN><ToString>PATH environment variable does not have C:\tools in it. Adding...</ToString><Props><Obj N="MessageData" RefId="3"><TN RefId="2"><T>System.Management.Automation.HostInformationMessage</T><T>System.Object</T></TN><ToString>PATH environment variable does not have C:\tools in it. Adding...</ToString><Props><S N="Message">PATH environment variable does not have C:\tools in it. Adding...</S><B N="NoNewLine">false</B><S N="ForegroundColor">Gray</S><S N="BackgroundColor">Black</S></Props></Obj><S N="Source">C:\Users\vagrant\chocolatey\helpers\functions\Install-ChocolateyPath.ps1</S><DT N="TimeGenerated">2024-04-19T11:54:45.2705814-07:00</DT><Obj N="Tags" RefId="4"><TN RefId="3"><T>System.Collections.Generic.List`1[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]</T><T>System.Object</T></TN><LST><S>PSHOST</S></LST></Obj><S N="User">CLIENTDB\vagrant</S><S N="Computer">clientdb</S><U32 N="ProcessId">1528</U32><U32 N="NativeThreadId">2804</U32><U32 N="ManagedThreadId">7</U32></Props></Obj><S S="verbose">Choosing not to expand environment names</S><S S="debug">Test-ProcessAdminRights: returning False</S><S S="debug">Running Start-ChocolateyProcessAsAdmin -statements 'Install-ChocolateyPath -pathToInstall 'C:\tools' -pathType 'Machine'' </S><S S="debug">Test-ProcessAdminRights: returning False</S><S S="Error">Attempted (unsuccessfully) to elevate recursively. Aborting..._x000D__x000A_</S><S S="Error">At C:\Users\vagrant\chocolatey\helpers\functions\Start-ChocolateyProcessAsAdmin.ps1:138 char:9_x000D__x000A_</S><S S="Error">+         throw "Attempted (unsuccessfully) to elevate recursively. Abo ..._x000D__x000A_</S><S S="Error">+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~_x000D__x000A_</S><S S="Error">    + CategoryInfo          : OperationStopped: (Attempted (unsu...ly. Aborting...:String) [], RuntimeException_x000D__x000A_</S><S S="Error">    + FullyQualifiedErrorId : Attempted (unsuccessfully) to elevate recursively. Aborting..._x000D__x000A_</S><S S="Error"> _x000D__x000A_</S></Objs>
ERROR: Running ["C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe" -NoLogo -NonInteractive -NoProfile -ExecutionPolicy Bypass -InputFormat Text -OutputFormat Text -EncodedCommand IAAgACAAIAAgACAAJABuAG8AUwBsAGUAZQBwACAAPQAgACQARgBhAGwAcwBlAA0ACgAgACAAIAAgACAAIAAjACQAZQBuAHYAOgBDAGgAbwBjAG8AbABhAHQAZQB5AEUAbgB2AGkAcgBvAG4AbQBlAG4AdABEAGUAYgB1AGcAPQAnAGYAYQBsAHMAZQAnAA0ACgAgACAAIAAgACAAIAAjACQAZQBuAHYAOgBDAGgAbwBjAG8AbABhAHQAZQB5AEUAbgB2AGkAcgBvAG4AbQBlAG4AdABWAGUAcgBiAG8AcwBlAD0AJwBmAGEAbABzAGUAJwANAAoAIAAgACAAIAAgACAAJgAgAGkAbQBwAG8AcgB0AC0AbQBvAGQAdQBsAGUAIAAtAG4AYQBtAGUAIAAnAEMAOgBcAFUAcwBlAHIAcwBcAHYAYQBnAHIAYQBuAHQAXABjAGgAbwBjAG8AbABhAHQAZQB5AFwAaABlAGwAcABlAHIAcwBcAGMAaABvAGMAbwBsAGEAdABlAHkASQBuAHMAdABhAGwAbABlAHIALgBwAHMAbQAxACcAIAAtAFYAZQByAGIAbwBzAGUAOgAkAGYAYQBsAHMAZQAgAHwAIABPAHUAdAAtAE4AdQBsAGwAOwANAAoAIAAgACAAIAAgACAAdAByAHkAewANAAoAIAAgACAAIAAgACAAIAAgACQAcAByAG8AZwByAGUAcwBzAFAAcgBlAGYAZQByAGUAbgBjAGUAPQAiAFMAaQBsAGUAbgB0AGwAeQBDAG8AbgB0AGkAbgB1AGUAIgANAAoAIAAgACAAIAAgACAAIAAgAEkAbgBzAHQAYQBsAGwALQBDAGgAbwBjAG8AbABhAHQAZQB5AFAAYQB0AGgAIAAtAHAAYQB0AGgAVABvAEkAbgBzAHQAYQBsAGwAIAAnAEMAOgBcAHQAbwBvAGwAcwAnACAALQBwAGEAdABoAFQAeQBwAGUAIAAnAE0AYQBjAGgAaQBuAGUAJwANAAoAIAAgACAAIAAgACAAIAAgAGkAZgAoACEAJABuAG8AUwBsAGUAZQBwACkAewBzAHQAYQByAHQALQBzAGwAZQBlAHAAIAA2AH0ADQAKACAAIAAgACAAIAAgAH0ADQAKACAAIAAgACAAIAAgAGMAYQB0AGMAaAB7AA0ACgAgACAAIAAgACAAIAAgACAAaQBmACgAIQAkAG4AbwBTAGwAZQBlAHAAKQB7AHMAdABhAHIAdAAtAHMAbABlAGUAcAAgADgAfQANAAoAIAAgACAAIAAgACAAIAAgAHQAaAByAG8AdwANAAoAIAAgACAAIAAgACAAfQA=] was not successful. Exit code was '1'. See log for possible error messages.
The install of add-path was NOT successful.
Error while running 'C:\Users\vagrant\chocolatey\lib\add-path\tools\chocolateyinstall.ps1'.
 See log for details.

Chocolatey installed 0/1 packages. 1 packages failed.
 See the log for details (C:\Users\vagrant\chocolatey\logs\chocolatey.log).

Failures
 - add-path (exited 1) - Error while running 'C:\Users\vagrant\chocolatey\lib\add-path\tools\chocolateyinstall.ps1'.
 See log for details.

Operating Systems Testing

Win10

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #3424

@vexx32 vexx32 requested a review from gep13 April 19, 2024 19:21
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Add a guard into Start-ChocolateyProcessAsAdmin to ensure we don't
accidentally recurse in the case of calling back into helper commands
that themselves call Start-ChocolateyProcessAsAdmin.
@gep13 gep13 force-pushed the 3424-elevation-guard branch from 46cb681 to 4c4ab3b Compare April 24, 2024 15:07
@gep13 gep13 merged commit 3813ac3 into chocolatey:develop Apr 24, 2024
5 checks passed
@gep13
Copy link
Member

gep13 commented Apr 24, 2024

@vexx32 thank you for getting this fixed up!

@vexx32 vexx32 deleted the 3424-elevation-guard branch April 29, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants