CreateProcessAsUser icon indicating copy to clipboard operation
CreateProcessAsUser copied to clipboard

Starting apps with administrative privileges in interactive sessions.

Open AbdulRaoufMKamal opened this issue 9 months ago • 4 comments

Previously, we couldn't load apps ui that require administrative privileges because of the fact that session ids being set to 0 (SYSTEM context) even though startInfo.lpDesktop = "winsta0\default" was set inside the method. Now, with the usage of SetTokenInformation along with providing TOKEN_ALL_ACCESS to the current process token ( the service) we can load the app UI inside the interactive session.

AbdulRaoufMKamal avatar Feb 19 '25 22:02 AbdulRaoufMKamal

Thanks for the submission @AbdulRaoufMKamal! As is mentioned in the README, I'm not in a great position to vet this, as I've long stepped away from Windows development. Before merging, I'd like to get a review from some other contributors, like @MV10, @0x53A, @mmiszczyk, or @KamenRiderKuuga.

murrayju avatar Feb 20 '25 17:02 murrayju

I admit I'm not immediately sure of TOKEN_ALL_ACCESS, but it seems to me that in this PR we are always starting the application with administrative privileges. I think best practice here would be to not do this by default, and only when explicitly requested - either through a different function (something like CreateProcessAsUserElevated), or by adding an optional boolean parameter to CreateProcessAsUser, defaulting to fault. This might seem like a minor thing because we're doing it from a perspective of a highly privileged service, but I think of it in context of principle of least privilege - kind of like with Linux daemons that work as root only for as long as they need to, and drop to a lower privilege level when they can.

Also, are changing versions of a supported .NET runtime and the WindowsServices library needed for this patch to work? If not, I think they should be a part of a different PR.

mmiszczyk avatar Feb 20 '25 18:02 mmiszczyk

Thank you for your comment @mmiszczyk. As for TOKEN_ALL_ACCESS I tried to take lesser access level (incrementally) to see which access level would break the method and throw an exception, and found that TokenAccessLevels.Write didn't work (the one just before TokenAccessLevels.AllAccess) so I had to stick with TOKEN_ALL_ACCESS (refer to this link for more info). Additionally, I have added a parameter called asAdmin to allow the dev to choose whether to run the process with admin privileges or not, and inside the method itself if asAdmin is false, then we wouldn't have to change token info ( the session ID as it already runs in session 1 or above). Moreover, when duplicating the token, we copy only the access rights of the existing token (by setting the second param to 0 as done before by @murrayju ).

AbdulRaoufMKamal avatar Mar 24 '25 22:03 AbdulRaoufMKamal

Made a small update in the LaunchProcess method; I have added a small check to detect whether the process running is being executed under the SYSTEM account or not (to see in which session the process currently runs) hence preventing the user from incorrectly identifying the account under which the process runs. @murrayju @mmiszczyk @MV10 @0x53A

AbdulRaoufMKamal avatar Mar 26 '25 20:03 AbdulRaoufMKamal