views:

640

answers:

7

We're currently building an application that executes a number of external tools. We often have to pass information entered into our system by users to these tools.

Obviously, this is a big security nightmare waiting to happen.

Unfortunately, we've not yet found any classes in the .NET Framework that execute command line programs while providing the same kind of guards against injection attacks as the IDbCommand objects do for databases.

Right now, we're using a very primitive string substitution which I suspect is rather insufficient:

protected virtual string Escape(string value)
{
      return value
        .Replace(@"\", @"\\")
        .Replace(@"$", @"\$")
        .Replace(@"""", @"\""")
        .Replace("`", "'")
      ;
}

What do you guys do to prevent command-line injection attacks? We're planning to implement a regex that is very strict and only allows a very small subset of characters through, but I was wondering if there was a better way.

Some clarifications:

  • Some of these tools do not have APIs we can program against. If they did, we wouldn't be having this problem.
  • The users don't pick tools to execute, they enter meta-data which the tools we've chosen use (for example, injecting meta data such as copyright notices into target files).
+4  A: 

Are you executing the programs directly or going through the shell? If you always launch an external program by giving the full path name to the executable and leaving the shell out of the equation, then you aren't really susceptible to any kind of command line injection.

EDIT: DrFloyd, the shell is responsible for evaluating things like the backtick. No shell, no shell evaluation. Obviously, you've still got to be aware of any potential security gotchas in the programs that you're calling -- but I don't think this question is about that.

Curt Hagenlocher
A: 

Well, if you can invoke the tools programmatically without the command line, that would probably be your best option. Otherwise, you could potentially execute the command line tool via a user that has absolutely no access to do anything (except perhaps a single directory that they can't do any harm with)... though that may end up breaking the tool, depending on what the tool does.

Just note, I have never had to face this problem, because I have never actually had to invoke a command line tool from an externally facing application where the tool requires input from the user.

Mike Stone
A: 

Hmmm...

It sounds like you have a list of valid commands that the users are able to execute. But you don't want them to execute them all.

You could try to take the actual command line and verify the file exists in the "safe" location at least.

You could also solve the problem with more interface, provide a drop down of commands and parameters they could use. It's more work on your end, but it ultimately helps the users.

DrFloyd5
A: 
DrFloyd5
+2  A: 

When you Process.Start a new process, supply the parameters in its Parameters argument instead of building the whole command line yourself.

Haven't got time for a proper test, but I think that should help guard it to some level.

Will test this out tomorrow.

EDIT: Ah, someone beat me to it again. But here's another point: Try using the Console.InputStream (can't remember exact name) to supply data instead of passing parameters, is that a possible solution? like fix the command so it reads from the CON device and you then supply the data via input stream instead.

chakrit
+1  A: 
Shadow2531
+1  A: 

Don't use a blacklist for preventing injections. If there are n ways to inject code, you'll think of n - m where m > 0.

Use a whitelist of accepted parameters (or patterns). It is much more restrictive by nature, but that's the nature of security.

craigb