Why you should never use '/bin/sh -c ...' in configuration files

March 4, 2013

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.

Comments on this page:

From at 2013-03-05 02:45:03:

The right way to handle SQL is to use placeholders, not escaping. Seriously, there is absolutely no way to get quoting correct, generically, in all cases. I'm kind of surprised that you're suggesting to escape things when placeholders have been around ever since I started doing DB stuff 10-12 years ago (and probably before then, even). Even PHP can do placeholders--though it doesn't make it easy.

... though I can clearly see my argument being made against the shell, too.

By cks at 2013-03-05 11:29:27:

The simple answer is that I usually consider placeholders to be a form of quoting, one that database APIs handle for you in your code.

(My understanding is that the actual implementation in some database communication protocols is more thorough than this, with the placeholders sent as separate entities from the actual SQL command.)

From at 2013-03-05 13:45:47:

My understanding is that most of the bigger databases have explicit support for placeholders in their protocols, with the notable exception being MySQL. Am I mistaken?

Aristotle Pagaltzis

By cks at 2013-03-05 14:11:45:

That's a good question and I don't know the answer; I haven't done any real research, especially on the current state of affairs. A quick check suggests that PostgreSQL and SQLite both support placeholders in their native interface/protocol; whether any particular language binding does or whether the code fakes things behind the scenes is an open question. Hopefully all of the language bindings that look like they support placeholders really do.

(The Python DB API doesn't require you to actually support real placeholders, just to look to the user like you do and then get quoting absolutely correct.)

To strain the analogy, the shell equivalent of placeholders is using separate arguments in exec() instead of relying on /bin/sh -c to parse things for you. It's not exact (since there are plenty of traps you can fall into when writing shell scripts), but it's probably the closest you can come.

And both commentators here are right about the right fix for SQL (and my mistake with it). I've updated the entry appropriately since getting this sort of thing right is important.

Written on 04 March 2013.
« Why a netcat-like program is a good test of a language
Turning off delays on failed password authentications »

Page tools: View Source, View Normal, Add Comment.
Login: Password:
Atom Syndication: Recent Comments.

Last modified: Mon Mar 4 23:55:31 2013
This dinky wiki is brought to you by the Insane Hackers Guild, Python sub-branch.