2013-03-04
Why you should never use '/bin/sh -c ...' in configuration files
Every so often some sysadmin has a problem configuring program A to run a command in just the right way. Maybe they decide they need to run multiple commands, or they need a complex command line, or there's any number of other reasons. So they decide to get out the big guns to solve their problem:
some_option = /bin/sh -c "CMD ARG 'arg 2' $somevar ....."
WRONG. This is a terrible idea (as I mentioned before).
The problem with solving your problems with /bin/sh this way is
that you have just thoroughly destroyed absolutely everything that
program A is possibly doing to let you securely run programs. It can be the most careful program
in the world, it can do absolutely everything right, and you have just
totally defeated all of its efforts. There is almost no way to make
this sort of command invocation secure in the face of malicious input,
and even trying moderately hard requires determined and careful manual
effort on your part. That $somevar variable expansion? That's a land
mine waiting to go off unless you both quote it and then make sure that
it doesn't have quote-breaking characters inside it.
Fundamentally this use of /bin/sh is exactly the sort of string
pasting anti-pattern that got everyone all of those SQL injection
attacks of the last N years. Everyone learned their lesson (we hope)
when it came to putting user input in SQL statements (and that lesson
is escape absolutely everything, carefully use
placeholders); we should not be repeating the same mistake when it
comes to shell command lines. Sadly, people do all the time.
(They get away with it in large part for the same reason that people got away with bad SQL habits; most input isn't malicious or accidentally dangerous.)
Theoretically, one answer to this problem is very carefully quoting every variable expansion and the like that goes into the shell command line. But that's a terrible answer in practice because it demands constant attention and vigilance on the part of everyone who writes this sort of configuration stanza (and it requires program A to support full and completely correct shell quoting in the first place). A much better answer, one that is much more resilient in the face of inevitable human error, is to let program A do its job of securely running simple commands and put all of the complexity in a shell script.
(I'm not even going to try to inventory the quoting bear traps that are waiting in such a shell command line. I'm not sure I could get them all and I don't want to encourage people by providing a list of workarounds and so on.)
Update: as correctly pointed out in comments the real SQL fix is to use proper SQL placeholders, which totally separate the SQL statement from the parameters and thus make it totally safe. See the comments for more.