rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.04k stars 879 forks source link

Invoking cargo completions in malicious project may lead to RCE #3740

Open bjorn3 opened 3 months ago

bjorn3 commented 3 months ago

A simple repro would be a project dir with the following contents:

rust-toolchain.toml:

[toolchain]
path = "/path/to/project"

bin/cargo (executable bit set):

#!/usr/bin/env bash
echo "Malicious code executed" > oops.txt

And that's it. If you now try to do shell completion in bash for "cargo check --bin " (trailing space important to avoid getting "--bin" and "--bins" as completions), a file called oops.txt will be created with "Malicious code executed" as content. (This needs the cargo bash completion to be enabled)

This RCE works because the cargo completion invokes cargo to get the list of bin targets. When using rustup, rustup will try to invoke the cargo of the active toolchain, which in this case is the project directory itself thanks to the rust-toolchain.toml file and as such bin/cargo in the project directory is invoked.

While it would be true that actually running the cargo command would allow running arbitrary code already, this is a lot more widely known and less surprising. I don't expect completions to have any side-effects like running malicious code in the POC.

This issue is mostly disclosed already in https://github.com/rust-lang/cargo/issues/6645#issuecomment-1889975423 as I didn't realize that the current state of the cargo completions already allow this. No POC has been posted there though.

For reference oh-my-zsh used to have a similar RCE opportunity by running "rustup run completions zsh cargo" whenever opening a new shell. This would run "cargo completions zsh" for the active toolchain of the directory in which the shell was opened. After I reported this, it was changed to use the default toolchain instead in https://github.com/ohmyzsh/ohmyzsh/commit/a01cf8562700f3e5594f66daf40f31d9fe7ec570

The POC is shared with the security team made use of path = "." which no longer works as of https://github.com/rust-lang/rustup/pull/3340. According to https://github.com/rust-lang/rustup/issues/3461 it should be possible to use /proc/self/cwd as workaround, but I couldn't get this to work on my machine. For a targeted attack it is likely still possible to guess the location where the victim will clone the malicious project.

rbtcollins commented 3 months ago

Since commands are consistent everywhere, one fix would be to change completions to also always use the default toolchain.

bjorn3 commented 3 months ago

That would cause issues when a project uses nightly cargo features but the default toolchain is stable. It is probably the safest option though. Oh-my-zsh does the same when for loading the completion script itself.