On Linux, using the std::process::Command API resolves commands using the $PATH environment variable. This represents a security decision, as by default only protected locations are included in this search path (/bin, /usr/bin, etc). It is possible for a user to change this behaviour and include additional search paths.
On Windows, at least with the current implementation, this security safeguard is overridden. Instead of only searching for the executable in @PATH@, the implementation looks for the binaries in the current path of the process first. This allows an attacker to hijack a Rust tool by placing a suitably named executable in the current working directory of a program that uses this API.
This has lead to CVE-2021-3013 (see cli: fix arbitrary execution of program bug · BurntSushi/ripgrep@229d1a8 · GitHub for the fix). In the fixing commit, basically the logic that UNIX (well, bash) uses to locate the binary is replicated, by parsing the PATH environment variable and looking for suitable matches in the directories, see the resolve_binary method.
Should some method be added to std::process::Command that resolves the name of an executable to an absolute path in a safe way by parsing the @PATH@ environment variable, in order to offer a simple way for code to be safe on Windows as well? Having some way to circumvent Window's defaults here and having this in the documentation will probably help everyone in writing safer, cross-platform code. Or maybe is there already something that does this and I was just not able to locate it?
Definitely there should be an option to add/remove cwd from the lookup path. With free reign, I'd say to "fix" Command to be consistent cross platform and not include cwd in the lookup path by default (assuming $env:PATH doesn't have . in it, because that's a bad idea, though legal), but given we're stable, matching the native shell environment is a decent enough choice to stick with.
I think the best API would be to provide a builder method that sets cwd lookup to one of { Native, Yes, No }, and a version of fs::canonicalize that only allows $PATH-relative or absolute names, not cwd-relative.
For a little context, cmd.exe does include cwd in the lookup chain for executables. I don't actually know whether MSDOS had a concept of $PATH, or if there were just shell builtins. For backwards compatibility, cmd.exe doesn't change, ever. MS even had difficulty finally splitting cmd.exe into the typical term / window architecture due to compatibility guarantees.
But the recommended Modern Windows approach is to use Powershell, which is shipped by default with the OS, or even install-requiring cross-platform Powershell inside Windows Terminal. No matter which version of PS you use, PS does not include cwd in executable path lookup, and requires ./ to execute a local executable. There's even a note printed when you fail to lookup a name in PATH but there is one locally, telling you to use ./ if you mean the local file.
So while including cwd in path lookup isn't default on Windows, it's still the behavior in the MS recommended shell environment, and as such something likely to be familiar to most Windows developers.
To be fair, three of those are basically a living history of where Windows has stored system binaries over the decades.
But yeah, by using lpApplicationName with an absolute path, we can skip the legacy win32 rules and implement our own behaviour however we like. This also allows running executables whose path exceeds MAX_PATH.
I was also tempted to replicate the PATH search to fix an issue with a bug in macOS's posix_spawnp implementation (see https://github.com/rust-lang/rust/pull/80537). I ended up doing a more conservative fix, though.
I'm surprised at this, as I thought CreateProcessW was limited to MAX_PATH no matter what (even with \\?\ paths). Or maybe it is lpCurrentDirectory that doesn't support MAX_PATH? Or maybe that needs a manifest?
Just an update to my thinking here after exploring the Windows code more thoroughly.
Changing how Command::new works by default may possibly be a breaking change? It might be necessary to deprecate it and replace it with something like Command::app. This would be almost the same except it would allow having much better defaults without breaking existing code.
It would also be nice if a resolve method or builder was able to configure exactly which paths are searched. E.g. system paths, parent PATH, child PATH, custom path(s). That said, I am worried about overcomplicating the API. But simply stating that it should work how Linux works has lead to problems in the past (both in terms of user expectations and platform differences).
Lastly I would like to simplify the way .exe is appended to the file name but this will definitely be a breaking change (e.g. it would break rustdoc). I think it should always be appended unless the file name already ends with .exe or the user explicitly opts out of this behaviour. It makes searching simpler and is, I think, the least surprising behaviour as well as the most consistent. The current behaviour is all over the map depending on a) the type of path that's used and b) whether or not any environment variables have been set (it doesn't matter which ones).
.exe is not the only executable extension though (see the PATHEXT environment variable).
As for the new name, app seems…inaccurate. Considering that the name means something specific on macOS, I'd question whether I need a .app bundle here or will any executable work. What is an "app" on Linux? How about Command::named(…) instead?
.exe is not the only executable extension though (see the PATHEXT environment variable).
Yeah, that's why there should be an opt-out. Note that in any case Command can only run actual PE executables (so not .bat, .js, etc). And will currently only append .exe (or nothing) and not any other extension.
Hmm, thinking about it some more I guess Rust could look up the file associations for the right command to run (it's stored in the registry). But I worry a bit about having such code in the standard library. It sounds more like a job for a third party crate.
Philosophically speaking, what is the intent of Command? Is it meant to work like a kind of shell? Or is just meant to be a thin wrapper to execute a program?
I see it as a way to execute a program in a reasonably cross-platform way. Of course, Unix can stuff all kinds of things between fork and exec to do things like drop permissions and other such things and Windows' CreateProcess has only a few dozen switches and knobs to twiddle with, but that kind of control belongs to crates that are way more platform-specific than the stdlib.
As for a shell, one can make a shell out of the bits Command provides (pipes, redirects, etc.), but there are a lot of details that matter there that the stdlib shouldn't do anything about other than make it possible (up to some level of cross-platform support).