# std::process::Command resolve() to avoid security issues on Windows?

**URL:** <https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800>\
**Category:** libs\
**Created:** [May 29, 2021, 3:52pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800 "2021-05-29T15:52:43Z")\
**Posts on this page:** 13\
**Page:** 1

<div class="post-metadata">

**Author:** ![xfbs](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/xfbs/32/8197_2.png) [@xfbs](https://internals.rust-lang.org/u/xfbs)\
**Post date:** [May 29, 2021, 3:52pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/1 "2021-05-29T15:52:43Z")

</div>

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](https://github.com/BurntSushi/ripgrep/commit/229d1a8d41b0023420e7815578fa0b39c0d5c2e4) 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?

---

<div class="post-metadata">

**Author:** ![CAD97](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cad97/32/3460_2.png) [@CAD97](https://internals.rust-lang.org/u/CAD97)\
**Post date:** [May 29, 2021, 4:23pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/2 "2021-05-29T16:23:24Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)\
**Post date:** [May 29, 2021, 4:34pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/3 "2021-05-29T16:34:20Z")

</div>

This is also how win32 [`CreateProcess`](https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw) works -- the `PATH` is **sixth** in the list of places that are searched.

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [May 29, 2021, 8:05pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/4 "2021-05-29T20:05:00Z")

</div>

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`.

* * *

EDIT: Looking at the implementation it already seems like we do some fun stuff: [rust/process.rs at ff5522fc1ae2c1d66fb2a465f48e03732fa8570b · rust-lang/rust · GitHub](https://github.com/rust-lang/rust/blob/ff5522fc1ae2c1d66fb2a465f48e03732fa8570b/library/std/src/sys/windows/process.rs#L165)

---

<div class="post-metadata">

**Author:** ![ehuss](https://avatars.discourse-cdn.com/v4/letter/e/9de0a6/32.png) [@ehuss](https://internals.rust-lang.org/u/ehuss)\
**Post date:** [May 30, 2021, 3:17pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/5 "2021-05-30T15:17:10Z")

</div>

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](https://github.com/rust-lang/rust/pull/80537)). I ended up doing a more conservative fix, though.

There may also be an interaction with [std::process::Command's current\_dir behaves differently on Unix and Windows, with respect to relative exe paths · Issue #37868 · rust-lang/rust · GitHub](https://github.com/rust-lang/rust/issues/37868), where it needs to decide how relative paths are resolved.

> [@chrisd](#):
>
> This also allows running executables whose path exceeds `MAX_PATH` .

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?

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [May 30, 2021, 3:19pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/6 "2021-05-30T15:19:02Z")

</div>

> [@ehuss](#):
>
> 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?

Yes `lpCurrentDirectory` is limited, as is the path in `lpCommandLine`. But `lpApplicationName` isn't limited.

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [June 22, 2021, 9:02pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/7 "2021-06-22T21:02:14Z")

</div>

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).

---

<div class="post-metadata">

**Author:** ![mathstuf](https://avatars.discourse-cdn.com/v4/letter/m/958977/32.png) [@mathstuf](https://internals.rust-lang.org/u/mathstuf)\
**Post date:** [June 22, 2021, 10:18pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/8 "2021-06-22T22:18:42Z")

</div>

`.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?

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [June 22, 2021, 10:23pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/9 "2021-06-22T22:23:54Z")

</div>

> `.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.

> How about `Command::named(…)`

Sure, I'm fine with bikeshedding the name.

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [June 22, 2021, 11:05pm UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/10 "2021-06-22T23:05:53Z")

</div>

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.

---

<div class="post-metadata">

**Author:** ![chrisd](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/chrisd/32/7232_2.png) [@chrisd](https://internals.rust-lang.org/u/chrisd)\
**Post date:** [June 23, 2021, 12:07am UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/11 "2021-06-23T00:07:45Z")

</div>

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?

---

<div class="post-metadata">

**Author:** ![mathstuf](https://avatars.discourse-cdn.com/v4/letter/m/958977/32.png) [@mathstuf](https://internals.rust-lang.org/u/mathstuf)\
**Post date:** [June 23, 2021, 1:19am UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/12 "2021-06-23T01:19:25Z")

</div>

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).

---

<div class="post-metadata">

**Author:** ![system](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/system/32/14092_2.png) [@system](https://internals.rust-lang.org/u/system)\
**Post date:** [September 21, 2021, 1:19am UTC](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800/13 "2021-09-21T01:19:57Z")

</div>

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.
