microsoft/sudo

`sudo -N` should preserve the current directory

gerardog opened this issue · 6 comments

gerardog commented

Sudo for Windows version

1.0.0

Windows build number

10.0.26063.1

Other Software

No response

Steps to reproduce

According to my tests, the general expectation regarding sudo is that it preserves the current directory.

If you run sudo rm * or sudo del *.* the user would expect to affect files in the current directory.
If you do sudo notepad ./filename, the user expects that the path resolves relative to the current directory.

But currently:

  • when -N is used
  • or sudo is configured in forceNewWindow mode

... then the current folder is set to C:\WINDOWS\System32 instead

  • Except if sudo is run from an already elevated console. (then it preserves the current directory)

For example

C:\test> del /P *.*
C:\Test\test-file.txt, Delete (Y/N)? Y
Access Denied

C:\test> sudo del /P *.*
C:\Test\test-file.txt, Delete (Y/N)? N

C:\test> sudo sudo config --enable forceNewWindow
Sudo mode set to Force New Window mode

C:\test> sudo del /P *.*
# a new window appears with:

C:\Windows\System32\07409496-a423-4a3e-b620-2cfb01a9318d_HyperV-ComputeNetwork.dll, Delete (Y/N)?

See:
image

I know you mentioned this was by-design. IMO that design choice should be reconsidered. This is not just unpredictable and inconvenient... In these edge cases it can also be dangerous.

Expected Behavior

Sudo should preserve the current directory, regardless of being invoked from an elevated process, a non-elevated one, with or without -N, and with any configuration mode.

Actual Behavior

Sudo preserves the current directory depending on the -N argument, the config mode, or the elevation status of the caller.

joadoumie commented

Alas, for security reasons, the default mode for sudo for windows has to avoid doing any RPC, which means it's going to basically just act as a shim for ShellExecute. You can avoid it by passing -D . to sudo (added in one of the more recent builds).

We're still iterating/ideating on ways to avoid this, but we don't have anything at the moment that also avoid RPC.

gerardog commented

Given that constraint, (and others, like SHELLEXECUTEINFO.lpDirectory being replaced to System32 when Verb=RunAs) I would then suggest you to elevate cmd /c cd %folder% & %command% to workaround those limitations.
(Or pushd which supports network paths)

zadjii-msft commented

You know, we had actually considered that one too. In that case, we then fall into the case where the UAC will show "Command Prompt" as the application being elevated, rather than the actual target application. I believe that was an undesirable outcome, though I'll loop back on that with the security folks.

I too agree, that solution would work, but I'm (very mildly) worried that less well-informed users will be surprised that sudo foo.exe will ask them to run "Command Prompt" rather than "Foo". Then again, if you're less-well-informed, you shouldn't be using sudo.

gerardog commented

For some reason, ShellExecuteEx with Verb=RunAs respects the starting directory if it is elevating dotnet apps but not for other types... (eg. sudo -N Pwsh.exe works but sudo -N PowerShell.exe doesn't). Any idea why that distinction is?

Is it possible that you guys alter ShellExecuteEx (in a backwards-compatible way) to respect the starting directory in every case?

... and, if the answer is no (or if sudo is running on older windows) then... I think users would find it more familiar to see a UAC popup for sudo.exe than an unexpected Command Prompt... So, maybe sudo -N foo could elevate itself, i.e: to elevate another sudo -D %path% foo. That way, the elevated sudo.exe won`t do any RPC since it is already elevated (will just CD and RUN)... And the UAC popup would show something familiar...

  • 👍1
zadjii-msft commented

(eg. sudo -N Pwsh.exe works but sudo -N PowerShell.exe doesn't). Any idea why that distinction is?

So, there's a horrifying thing about ShellExecute("runas", .... If the OS determines that the thing that's being launched lives in the Windows dir (system32 included), then it manually ignores the CWD, and always starts it in system32. Since powershell is in system32, and pwsh isn't, they act differently here:
image

We looked into this... a couple months back? The code that's responsible for doing that is deep. For my own reference (so I don't have to dig through Teams again), the code responsible starts with a big ol' comment:

    // For elevated EXEs in the Windows directory, make sure
    // the current directory cannot be overriden to an insecure
    // location.  ...

(deep in AiLaunchProcess)

.... and it looks like that code dates back to 2006, in Vista.

BUG: 1844796: LUA: Reserve "Windows binary" marking in the elevation UI for EXEs launched from known Windows locations

Looks like that was multiple bug databases ago, so I'm not sure I can get farther back into the details than that.

  • 😄1
Masamune3210 commented

Alas, for security reasons, the default mode for sudo for windows has to avoid doing any RPC, which means it's going to basically just act as a shim for ShellExecute. You can avoid it by passing -D . to sudo (added in one of the more recent builds).

We're still iterating/ideating on ways to avoid this, but we don't have anything at the moment that also avoid RPC.

Ok, but.......Linux has been doing it this way since almost the start. Why can't Windows?

  • 😕1