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

(notepadplusplus) Automatic restart of running application during upgrade always launches new Notepad++ process with elevated privileges as user who ran choco upgrade regardless of original running user #2334

Closed
blkeller opened this issue Oct 21, 2023 · 8 comments · Fixed by #2566
Assignees
Labels
0 - Backlog 4 - Done Blocked: Upstream Used on discussions and issues when implementation is blocked by upstream softwares Bug Security / CVE

Comments

@blkeller
Copy link

blkeller commented Oct 21, 2023

Chocolatey Version

2.2.2

Chocolatey License

None

Package Version

8.5.7 -> 8.5.8

Current Behaviour

When running choco upgrade notepadplusplus and an existing instance of Notepad++ is running, the package logic shuts down the running process, then performs the upgrade, then relaunches Notepad++ in an attempt to bring back the user's Notepad++ session using the newly updated version of the Notepad++ executable. The restart logic, however, does not consider which user was running the application before shutting it down, and the simple Start-Process directive launches the new instance of Notepad++ in such a way that the new process inherits both the user and the elevated privilege of the Chocolatey process running choco upgrade.

Expected Behaviour

When the Chocolatey package relaunches Notepad++, the spawned process should run as the original user who was running it before the package upgrade logic shut it down. Also, Chocolatey should not escalate privileges for the new process if the old process was running with regular user permissions (which is most commonly how Notepad++ is run).

Steps To Reproduce

This can fail in several interesting ways:

Case 1:

  1. The MyAdmin user is logged into Windows and running Notepad++ in their own user session without elevated permissions.
  2. In Notepad++, MyAdmin attempts to save a new file or modify an existing file under C:\Windows\, and they receive an error about not having the proper permissons, as expected.
  3. MyAdmin runs Command Prompt as Administrator (with elevated permissions) where they execute choco upgrade notepadplusplus, and the upgrade completes normally, including shutting down and relaunching Notepad++.
  4. Note that in the newly launched Notepad++ window, the titlebar is now suffixed with "[Administrator]".
  5. Repeat Step 2, but now MyAdmin is able to successfully save the file without any permission errors.

Case 2:

  1. The OtherGuy user is logged into Windows and running Notepad++ in their own user session without elevated permissions.
  2. Keeping OtherGuy logged in, tell Windows to switch users to MyAdmin.
  3. MyAdmin runs Task Manager and verifies on the "Details" tab that Notepad++ is still running and that the running user is OtherGuy.
  4. In their own user session, MyAdmin runs Command Prompt as Administrator (with elevated permissions) where they execute choco upgrade notepadplusplus, and the upgrade completes normally, including shutting down and relaunching Notepad++.
  5. Note that a new Notepad++ window has opened in MyAdmin's Windows user session. Also note that the titlebar is suffixed with "[Administrator]".
  6. Switch users again back to OtherGuy.
  7. Note that there is no longer any running instance of Notepad++ in OtherGuy's Windows user session like there was before the upgrade.

Case 3:

  1. The LimitedUser user is logged into Windows and running Notepad++ in their own user session without elevated permissions.
  2. In the same user session, we run Command Prompt as Administrator, authenticating as MyAdmin (with elevated permissions).
  3. In that command prompt, run tasklist /V /FI "ImageName eq notepad++.exe" /FO LIST and note that under "User Name" the user running Notepad++ is shown as LimitedUser.
  4. In the same command prompt, MyAdmin executes choco upgrade notepadplusplus, and the upgrade completes normally, including shutting down and relaunching Notepad++.
  5. Note that the new Notepad++ window opened in LimitedUser's Windows session, which is expected.
  6. In the same command prompt, run tasklist /V /FI "ImageName eq notepad++.exe" /FO LIST and note that under "User Name" the user running Notepad++ is now shown as MyAdmin, which is not expected.
  7. MyAdmin closes their elevated command prompt window, hands control back of the machine back to LimitedUser, and walks away from the computer, thinking everything is OK.
  8. LimitedUser starts using the instance of Notepad++ running within their Windows session and notices there is a new suffix of "[Administrator]" in the titlebar.
  9. LimitedUser uses Notepad++ to create or modify system files that they otherwise don't have permission to change, e.g. C:\Windows\System32\drivers\etc\hosts.

Environment

  • Operating System: Microsoft Windows NT 10.0.19044.0 (Windows 10)

  • PowerShell Version:

Name                           Value
----                           -----
PSVersion                      5.1.19041.3570
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.19041.3570
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
  • Shell: Command Prompt

Chocolatey Log

https://gist.github.com/blkeller/66cfbf75a89d8dd9c1b60726f92011ba

Anything else?

No response

@blkeller blkeller added the Bug label Oct 21, 2023
@pauby
Copy link
Member

pauby commented Oct 24, 2023

Is this something you will be submitting a PR for?

@blkeller
Copy link
Author

No, sorry, I did not have any immediate plans to write a patch for this myself. I'm not an everyday user of Chocolatey (yet), but my company is evaluating it for possible organizational use. As part of that evaluation, I was going through the training material and documentation, which uses the notepadplusplus package from the community repository as the example, so that's why I started poking, prodding, and experimenting with this package in particular. (And yes, we will be using our own internal repository if we decide to move forward with adopting Chocolatey as an organization.) Having encountered this issue during my testing, I felt a responsibility to at least file what I hope is a useful bug report.

@AdmiringWorm
Copy link
Member

This does sound like something that would be useful to fix/support, not just for the Notepad++ package.

As such, I think we need to update one of our extensions with support of running a process as non-elevated, and then update the Notepad++ package to call this extension CMDLET.

As such, I have created chocolatey-community/chocolatey-extensions#15 to track this work.

Copy link

github-actions bot commented Jan 6, 2024

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
This issue will be closed in 14 days if it continues to be inactive.

Please do not add a comment to circumvent automatic closure unless you plan to help move it forward.
Doing this may lead to the issue being closed immediately instead.

Copy link

Dear contributor,

As this issue seems to have been inactive for quite some time now, I've automatically closed it.
If you feel this is a valid issue, please feel free to re-open the issue if/when a pull request
has been added.
Thank you for your contribution.

@blkeller
Copy link
Author

Since this issue is pending chocolatey-community/chocolatey-extensions#15, shouldn't it remain open?

@AdmiringWorm
Copy link
Member

Normally I would probably say no, but I agree in this case that we should probably leave this issue open.

It is basically being blocked until support is added in the chocolatey-core.extension package first.

@AdmiringWorm AdmiringWorm reopened this Jan 24, 2024
@AdmiringWorm AdmiringWorm added Blocked: Upstream Used on discussions and issues when implementation is blocked by upstream softwares 0 - Backlog and removed Pending closure Unresolved labels Jan 24, 2024
@pauby
Copy link
Member

pauby commented Nov 7, 2024

Until the work is done in chocolatey-core.extension this behaviour needs to be removed as it has security implications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Backlog 4 - Done Blocked: Upstream Used on discussions and issues when implementation is blocked by upstream softwares Bug Security / CVE
Projects
None yet
3 participants