18

From findutils' manual:

For example constructs such as these two commands

# risky
find -exec sh -c "something {}" \;
find -execdir sh -c "something {}" \;

are very dangerous. The reason for this is that the ‘{}’ is expanded to a filename which might contain a semicolon or other characters special to the shell. If for example someone creates the file /tmp/foo; rm -rf $HOME then the two commands above could delete someone’s home directory.

So for this reason do not run any command which will pass untrusted data (such as the names of fi les) to commands which interpret arguments as commands to be further interpreted (for example ‘sh’).

In the case of the shell, there is a clever workaround for this problem:

# safer
find -exec sh -c 'something "$@"' sh {} \;
find -execdir sh -c 'something "$@"' sh {} \;

This approach is not guaranteed to avoid every problem, but it is much safer than substituting data of an attacker’s choice into the text of a shell command.

  1. Is the cause of the problem in find -exec sh -c "something {}" \; that the replacement for {} is unquoted and therefore not treated as a single string?
  2. In the solution find -exec sh -c 'something "$@"' sh {} \;,

    • first {} is replaced, but since {} is unquoted, doesn't "$@" also have the same problem as the original command? For example, "$@" will be expanded to "/tmp/foo;", "rm", "-rf", and "$HOME"?

    • why is {} not escaped or quoted?

  3. Could you give other examples (still with sh -c, or without it if applicable; with or without find which may be not necessary) where the same kind of problem and solution apply, and which are minimal examples so that we can focus on the problem and solution with little distraction as possible? See Ways to provide arguments to a command executed by `bash -c`

Thanks.

29

This isn’t really related to quoting, but rather to argument processing.

Consider the risky example:

find -exec sh -c "something {}" \;
  • This is parsed by the shell, and split into six words: find, -exec, sh, -c, something {} (no quotes any more), ;. There’s nothing to expand. The shell runs find with those six words as arguments.

  • When find finds something to process, say foo; rm -rf $HOME, it replaces {} with foo; rm -rf $HOME, and runs sh with the arguments sh, -c, and something foo; rm -rf $HOME.

  • sh now sees -c, and as a result parses something foo; rm -rf $HOME (the first non-option argument) and executes the result.

Now consider the safer variant:

find -exec sh -c 'something "$@"' sh {} \;
  • The shell runs find with the arguments find, -exec, sh, -c, something "$@", sh, {}, ;.

  • Now when find finds foo; rm -rf $HOME, it replaces {} again, and runs sh with the arguments sh, -c, something "$@", sh, foo; rm -rf $HOME.

  • sh sees -c, and parses something "$@" as the command to run, and sh and foo; rm -rf $HOME as the positional parameters (starting from $0), expands "$@" to foo; rm -rf $HOME as a single value, and runs something with the single argument foo; rm -rf $HOME.

You can see this by using printf. Create a new directory, enter it, and run

touch "hello; echo pwned"

Running the first variant as follows

find -exec sh -c "printf \"Argument: %s\n\" {}" \;

produces

Argument: .
Argument: ./hello
pwned

whereas the second variant, run as

find -exec sh -c 'printf "Argument: %s\n" "$@"' sh {} \;

produces

Argument: .
Argument: ./hello; echo pwned
  • In the safer variant, why is {} not escaped or quoted? – Tim Jun 7 '18 at 17:31
  • 1
    It generally doesn’t need to be quoted; it would only need to be quoted in a shell where it has some other meaning, and I don’t think that’s the case with any of the main shells in use nowadays. – Stephen Kitt Jun 7 '18 at 18:19
  • From gnu.org/software/findutils/manual/html_mono/…: "Both of these constructions (; and {}) need to be escaped (with a ‘\’) or quoted to protect them from expansion by the shell." Do you mean that it is incorrect for bash? – Tim Jun 7 '18 at 18:40
  • 3
    I mean that it is incorrect for shells in general. See POSIX. That particular statement in the findutils manual dates back to at least 1996... Of course it does no harm to quote {}, either as '{}' or "{}", but it isn’t necessary. – Stephen Kitt Jun 7 '18 at 19:11
  • @Tim You're experiencing a common situation where your intuition isn't yet accounting for the fact that there are two very different types of argument-processing going on here: when/how the shell parses arguments out of a line of text (which is where quoting matters) is different from raw operating system argument passing (where quotes are just regular characters). In the shell command find some/path -exec sh -c 'something "$@"' {} \; there's actually three layers of argument processing/passing, two of the shell variety and one of the basic raw operating system variety. – mtraceur Jun 7 '18 at 22:03
3

Part 1:

find just uses text replacement.

Yes, if you did it unquoted, like this:

find . -type f -exec sh -c "echo {}" \;

and an attacker was able to create a file called ; echo owned, then it would exec

sh -c "echo ; echo owned"

which would result in the shell running echo then echo owned.

But if you added quotes, the attacker could just end your quotes then put the malicious command after it by creating a file called '; echo owned:

find . -type f -exec sh -c "echo '{}'" \;

which would result in the shell running echo '', echo owned.

(if you swapped the double quotes for single quotes, the attacker could use the other type of quotes too.)


Part 2:

In find -exec sh -c 'something "$@"' sh {} \;, the {} is not initially interpreted by the shell, it's executed directly with execve, so adding shell quotes wouldn't help.

find -exec sh -c 'something "$@"' sh "{}" \;

has no effect, since the shell strips the double quotes before running find.

find -exec sh -c 'something "$@"' sh "'{}'" \;

adds quotes that the shell doesn't treat specially, so in most cases it just means the command won't do what you want.

Having it expand to /tmp/foo;, rm, -rf, $HOME shouldn't be a problem, because those are arguments to something, and something probably doesn't treat its arguments as commands to execute.


Part 3:

I assume similar considerations apply for anything that takes untrusted input and runs it as (part of) a command, for example xargs and parallel.

  • xargs is only dangerous if -I or -J is used. In normal operation it's only appending arguments to the end of the list, just as -exec ... {} + does. – Charles Duffy Jun 7 '18 at 15:55
  • 1
    (parallel, by contrast, runs a shell by default without explicit request from the user, increasing the vulnerable surface; this is a matter that's been the subject of much discussion; see unix.stackexchange.com/questions/349483/… for an explanation, and the included link to lists.gnu.org/archive/html/bug-parallel/2015-05/msg00005.html). – Charles Duffy Jun 7 '18 at 16:02
  • @CharlesDuffy You mean "decreasing the vulnerable surface". The attack illustrated by OP does indeed not work by default with GNU Parallel. – Ole Tange Jun 8 '18 at 7:22
  • 1
    Yes, I know you need it for parity across SSH -- if you didn't spawn a remote parallel "receiver" process on the other end of any socket, able to do a direct shell-less execv. Which is exactly what I would have done, if in your shoes implementing the tool. Having a noninteractive program behave differently based on the current value of SHELL is hardly least-astonishment. – Charles Duffy Jun 8 '18 at 12:07
  • 1
    Yes -- I hold that pipes and redirections should be impossible unless the user explicitly starts a shell, at which point they get only the explicit behavior of the shell they explicitly started. If one can expect only what an execv provides, that's simpler and less astonishing, and a rule that doesn't change across locations/runtime environments. – Charles Duffy Jun 8 '18 at 13:01
3

1. Is the cause of the problem in find -exec sh -c "something {}" \; that the replacement for {} is unquoted and therefore not treated as a single string?

In a sense, but quoting cannot help here. The filename that gets replaced in place of {} can contain any characters, including quotes. Whatever form of quoting was used, the filename could contain the same, and "break out" of the quoting.

2. ... but since {} is unquoted, doesn't "$@" also have the same problem as the original command? For example, "$@" will be expanded to "/tmp/foo;", "rm", "-rf", and "$HOME"?

No. "$@" expands to the positional parameters, as separate words, and doesn't split them further. Here, {} is an argument to find in itself, and find passes the current filename also as a distinct argument to sh. It's directly available as a variable in the shell script, it's not processed as a shell command itself.

... why is {} not escaped or quoted?

It doesn't need to be, in most shells. If you run fish, it needs to be: fish -c 'echo {}' prints an empty line. But it doesn't matter if you quote it, the shell will just remove the quotes.

3. Could you give other examples...

Any time you expand a filename (or another uncontrolled string) as-is inside a string that's taken as some kind of code(*), there's a possibility of arbitrary command execution.

For example, this expands $f directly the Perl code, and will cause problems if a filename contains a double quote. The quote in the filename will end the quote in the Perl code, and the rest of the filename can contain any Perl code:

touch '"; print "HELLO";"'
for f in ./*; do
    perl -le "print \"size: \" . -s \"$f\""
done

(The filename has to be a bit weird since Perl parses the whole code up front, before running any of it. So we'll have to avoid a parse error.)

While this passes it safely through an argument:

for f in ./*; do
    perl -le 'print "size: " . -s $ARGV[0]' "$f"
done

(It doesn't make sense to run another shell directly from a shell, but if you do, it's similar to the find -exec sh ... case)

(* some kind of code includes SQL, so obligatory XKCD: https://xkcd.com/327/ plus explanation: https://www.explainxkcd.com/wiki/index.php/Little_Bobby_Tables )

1

Your concern is precisely the reason why GNU Parallel quotes input:

touch "hello; echo pwned"
find . -print0 | parallel -0 printf \"Argument: %s\\n\" {}

This will not run echo pwned.

It will run a shell, so that if you extend your command, you will not suddenly get a surprise:

# This _could_ be run without spawining a shell
parallel "grep -E 'a|bc' {}" ::: foo
# This cannot
parallel "grep -E 'a|bc' {} | wc" ::: foo

For further details on the spawning-a-shell issue see: https://www.gnu.org/software/parallel/parallel_design.html#Always-running-commands-in-a-shell

  • 1
    ...that said, find . -print0 | xargs -0 printf "Argument: %s\n" is just as safe (or rather, moreso, since with -print0 one handles filenames with newlines correctly); parallel's quoting is a workaround for a problem that doesn't exist at all when no shell is present. – Charles Duffy Jun 8 '18 at 12:05
  • But as soon as you add | wc to the command you need to jump through hoops to make it safe. GNU Parallel is safe by default. – Ole Tange Jun 9 '18 at 14:27
  • ITYM: it tries to cover for every quirk of the shell language by the complicated process of carefully quoting everything. That's not nearly as safe as properly storing data in variables, not intermixed with code. Now, if it were to automatically convert that foo {} | wc into sh -c 'foo "$1" | wc sh {}`, that might be different. – ilkkachu Jun 10 '18 at 9:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.