Skip to content

Command's Debug impl has incorrect shell escaping #51198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
comex opened this issue May 30, 2018 · 3 comments
Open

Command's Debug impl has incorrect shell escaping #51198

comex opened this issue May 30, 2018 · 3 comments
Labels
A-fmt Area: `core::fmt` C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@comex
Copy link
Contributor

comex commented May 30, 2018

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
Copy link
Member

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
Copy link
Member

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

@estebank estebank added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 4, 2018
@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Aug 13, 2019
@Enselic Enselic added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 24, 2023
@Enselic Enselic added the A-fmt Area: `core::fmt` label Nov 12, 2024
@Enselic
Copy link
Member

Enselic commented Nov 12, 2024

Also see #114583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fmt Area: `core::fmt` C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants