views:

366

answers:

5

I have a Django application I'm developing that must make a system call to an external program on the server. In creating the command for the system call, the application takes values from a form and uses them as parameters for the call. I suppose this means that one can essentially use bogus parameters and write arbitrary commands for the shell to execute (e.g., just place a semicolon and then rm -rf *).

This is bad. While most users aren't malicious, it is a potential security problem. How does one handle these potential points of exploit?

EDIT (for clarification): The users will see a form that is split up with various fields for each of the parameters and options. However some fields will be available as open text fields. All of these fields are combined and fed to subprocess.check_call(). Technically, though, this isn't separated too far from just handing the users a command prompt. This has got to be fairly common, so what do other developers do to sanitize input so that they don't get a Bobby Tables.

+3  A: 

The answer is, don't let users type in shell commands! There is no excuse for allowing arbitrary shell commands to be executed.

Also, if you really must allow users to supply arguments to external commands, don't use a shell. In C, you could use execvp() to supply arguments directly to the command, but with django, I'm not sure how you would do this (but I'm sure there is a way). Of course, you should still do some argument sanitation, especially if the command has the potential to cause any harm.

Zifre
+1  A: 

Depending on the range of your commands, you could customize the form, so that parameters are entered in separate form-fields. Those you can parse for fitting values more easily. Also, beware of backticks and other shell specific stuff.

Ulf
+9  A: 

Based on my understanding of the question, I'm assuming you aren't letting the users specify commands to run on the shell, but just arguments to those commands. In this case, you can avoid shell injection attacks by using the subprocess module and not using the shell (i.e. specify use the default shell=False parameter in the subprocess.Popen constructor.

Oh, and never use os.system() for any strings containing any input coming from a user.

Rick Copeland
Reading through http://blog.littleimpact.de/index.php/2008/08/11/avoiding-shell-injection-in-ruby-python-and-php/ it seems that I'll be okay so long as shell=False, which is default. (That is, one, and only one command will be run.) Is this also what you're saying?
gotgenes
Yes, that's what I'm saying.
Rick Copeland
+5  A: 

By never trusting users. Any data coming from the web browser should be considered tainted. And absolutely do not try to validate the data via JS or by limiting what can be entered in the FORM fields. You need to do the tests on the server before passing it to your external application.

Update after your edit: no matter how you present the form to users on your front-end the backend should treat it as though it came from a set of text boxes with big flashing text around them saying "insert whatever you want here!"

Michael Cramer
Good point. I don't assume the client will be responsible and validate; I'll do validation on the server side before running the command. It's worth your stating, though, for the benefit of others.
gotgenes
+4  A: 

To do this, you must do the following. If you don't know what "options" and "arguments" are, read the optparse background.

Each "Command" or "Request" is actually an instance of a model. Define your Request model with all of the parameters someone might provide.

  1. For simple options, you must provide a field with a specific list of CHOICES. For options that are "on" or "off" (-x in the command-line) you should provide a CHOICE list with two human-understandable values ("Do X" and "Do not do X".)

  2. For options with a value, you must provide a field that takes the option's value. You must write a Form with the validation for this field. We'll return to option value validation in a bit.

  3. For arguments, you have a second Model (with an FK to the first). This may be as simple as a single FilePath field, or may be more complex. Again, you may have to provide a Form to validate instances of this Model, also.

Option validation varies by what kind of option it is. You must narrow the acceptable values to be narrowest possible set of characters and write a parser that is absolutely sure of passing only valid characters.

Your options will fall into the same categories as the option types in optparse -- string, int, long, choice, float and complex. Note that int, long, float and complex have validation rules already defined by Django's Models and Forms. Choice is a special kind of string, already supported by Django's Models and Forms.

What's left are "strings". Define the allowed strings. Write a regex for those strings. Validate using the regex. Most of the time, you can never accept quotes (", ' or `) in any form.

Final step. Your Model has a method which emits the command as a sequence of strings all ready for subprocess.Popen.


Edit

This is the backbone of our app. It's so common, we have a single Model with numerous Forms, each for a special batch command that gets run. The Model is pretty generic. The Forms are pretty specific ways to build the Model object. That's the way Django is designed to work, and it helps to fit with Django's well-thought-out design patterns.

Any field that is "available as open text fields" is a mistake. Each field that's "open" must have a regex to specify what is permitted. If you can't formalize a regex, you have to rethink what you're doing.

A field that cannot be constrained with a regex absolutely cannot be a command-line parameter. Period. It must be stored to a file to database column before being used.


Edit

Like this.

class MySubprocessCommandClass( models.Model ):
    myOption_1 = models.CharField( choice = OPTION_1_CHOICES, max_length=2 )
    myOption_2 = models.CharField( max_length=20 )
    etc.
    def theCommand( self ):
        return [ "theCommand", "-p", self.myOption_1, "-r", self.myOption_2, etc. ]

Your form is a ModelForm for this Model.

You don't have to save() the instances of the model. We save them so that we can create a log of precisely what was run.

S.Lott
Thanks for your answer. Rick Copeland's answer indicates there's safety included in the subprocess module in that, so long as shell=False, one and only one command will be executed; all other parts following the initial command are considered as arguments to the command. Since I control the one command at the beginning, do I need to really design regexps for the arguments?
gotgenes
Also, I'm curious, why is a Model necessary? I'm having a hard time placing everything together as you stated it. Our form is already represented as a subclass of forms.Form; we're using that for validation at the moment. How does this tie in to a model. Is the command you're referring to an actual (Base)Command class, from django.core.management.base?
gotgenes
Not anything to do with the Management.Command hierarchy. No, this Command is entirely separate.
S.Lott
Many thanks for the clarification via example code! This helps tremendously.
gotgenes