rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.36k stars 12.59k forks source link

Command's Debug impl has incorrect shell escaping #51198

Open comex opened 6 years ago

comex commented 6 years ago

On Unix, the Debug impl for Command prints the command using quotes around each argument, e.g.

"ls" "-la" "\"foo \""

The use of spaces as a delimiter suggests that the output is suitable to be passed to a shell. While it's debatable whether users should be depending on any specific debug representation, in practice, at least rustc itself uses it for user-facing output (when passing -Z print-link-args).

There are two problems with this:

  1. It's insecure! The quoting is performed, via the Debug impl for CStr, by ascii::escape_default, whose escaping rules are

    chosen with a bias toward producing literals that are legal in a variety of languages, including C++11 and similar C-family languages.

    However, this does not include escaping the characters $, `, and !, which have a special meaning within double quotes in Unix shell syntax. So, for example:

    let mut cmd = Command::new("foo");
    cmd.arg("`echo 123`");
    println!("{:?}", cmd);

    prints

    "foo" "`echo 123`"

    but if you run that in a shell, it won't produce the same behavior as the original command.

  2. It's noisy. In a long command line like those produced by -Z print-link-args, most arguments don't contain any characters that need to be quoted or escaped, and the output is long enough without unnecessary quotes.

Cargo uses the shell-escape crate for this purpose; perhaps that code can be copied into libstd.

retep998 commented 6 years ago

What about non-bash style shells like cmd or powershell? Are we just going to play favorites and ignore how escaping is handled in cmd and powershell?

zackmdavis commented 6 years ago

(Command's Debug output has also been criticized on other grounds: #42200)