Repository: rust
Issue: Insecure behavior in std::process::Command on Windows
Labels: O-windows,A-security,P-high,C-bug,T-libs,A-process

There appears to be a security vulnerability in std::process::Command, specifically on Windows.  The documentation for the `new` function states this:

> If program is not an absolute path, the PATH will be searched in an OS-defined way.
>
> The search path to be used may be controlled by setting the PATH environment variable on the Command, but this has some implementation limitations on Windows (see issue #37519).

What this does not say is that these implementation limitations cause the program to be executed from the current directory, even if that is not in `PATH`.  This is a vulnerability and this behavior has been known to be insecure on Unix for many years.

As a result, it is not possible to use Rust to invoke other than an absolute path when the current directory is untrusted, such as when a user is working in cloned Git repository.  Moreover, this fact is not even documented, and as such, I would reasonably expect that directories outside of `PATH` are not searched.

I've attached [a tarball of a cargo project](https://github.com/rust-lang/rust/files/6971445/path-check.tar.gz) which contains two trivial programs.  To reproduce the problem, do the following:

1. Open PowerShell.
2. Extract the tarball.
3. Change into `path-check`.
4. Run `cargo build`.
5. Change into `target\debug`.
6. Run `$env:Path = "C:\Users\User\.cargo\bin"` to set a fixed `PATH` without the current directory.
6. Run `& '.\path-check.exe'`.
7. Notice that `exploit.exe` is also invoked and that its output is displayed.

I used a [Windows 10 Development VM](https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/) for this purpose.  I don't habitually use Windows; I'm mostly a Linux user, so my apologies if the steps are hard to understand.

I realize that the current functionality exists because Rust looks only for `.exe` files and not other types of files, and it therefore it otherwise passes files which do not exist in `PATH` to `CreateProcessW`, which has the insecure behavior.  However, just because Microsoft has designed an insecure interface does not mean Rust should permit the same behavior.

[The approach that Go uses for searching for executables](https://github.com/golang/go/blob/master/src/os/exec/lp_windows.go), _other than the use of the current directory_, is generally good.  It uses `PATHEXT` (or, if absent, a hardcoded list) to look for extensions, and then considers each component in `PATH` (rejecting empty components), looking for each extension in turn.  This algorithm (with the current directory) is used by CMD, and therefore doing the same thing without the current directory would be normal and expected for Windows users, and also secure.

Note that unlike on Unix, on Windows empty components in `PATH` should not be treated as the current directory.  Unfortunately, many Windows machines contain a trailing semicolon in `PATH` and as such, that would preserve insecure semantics.

I originally reported this to the private security list, but it was [determined that this behavior had been discussed publicly before](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800), and thus, opening an issue was appropriate.  It remains a vulnerability, and a CVE should still be issued, though.

This came to my attention because Go has the same insecure behavior and they have deliberately chosen to retain the vulnerability for compatibility, so every Go program that runs on Windows (including Git LFS, which I maintain) must contain special code to work around this.  Since Rust has already documented secure behavior (using `PATH`), all that needs to be done is actually fixing the implementation, which is less of a problem.

### Meta

`rustc --version`:
```
rustc 1.54.0 (a178d0322 2021-07-26)
```

I've verified that the vulnerable code exists in a recent HEAD.