IDS34-PL. Do not pass untrusted, unsanitized data to a command interpreter

External programs are commonly invoked to perform a function required by the overall system. This is a form of reuse and might even be considered a crude form of component-based software engineering. Command and argument injection vulnerabilities occur when an application fails to sanitize untrusted input and uses it in the execution of external programs.

The exec() builtin function is the standard mechanism for executing system commands; it is a wrapper around the POSIX exec family of system calls. The system() builtin function is similar to exec, but it takes a single string whereas exec()takes a list. The qx() function, often represented by encasing a command in backquotes (``), can also be used to execute an arbitrary command. Finally the open() function can also execute commands in a subprocess and either send data to them or fetch data from them (but not both).

Command injection attacks cannot succeed unless a command interpreter is explicitly invoked. However, argument injection attacks can occur when arguments have spaces, double quotes, and so forth, or start with a - or / to indicate a switch.

This rule is a specific instance of rule IDS33-PL. Sanitize untrusted data passed across a trust boundary. Any string data that originates from outside the program's trust boundary must be sanitized before being executed as a command on the current platform.

Noncompliant Code Example (open())

This noncompliant code example tries to list a directory specified by the user. It safely uses the 3-argument open() command, as required by IDS31-PL. Do not use the two-argument form of open().

my $dir = $ARGV[0];
open( my $listing, "-|", "ls -F $dir") or croak "error executing command: stopped";
while (<$listing>) {
  print "Result: $_";
}
close( $listing);

The program also works properly when given a valid directory as an argument:

% ./sample.pl ~
Result: bin/
Result: Desktop/
Result: src/
Result: workspace/
%

But it can also have unintended consequences, as in this case, an attacker injects an arbitrary command to be executed by the call to open():

% ./example.pl "dummy ; echo bad"
ls: cannot access dummy: No such file or directory
Result: bad
% ./example.pl 

Compliant Solution (Sanitization)

This compliant solution sanitizes the untrusted user input by permitting only a small group of whitelisted characters in the argument that will be passed to open(); all other characters are excluded.

my $file;
my $dir = $ARGV[0];
croak "Argument contains unsanitary characters, stopped" if ($dir =~ m|[^-A-Za-z0-9_/.~]|);
open( my $listing, "-|", "ls -F $dir") or croak "error executing command: stopped";
while (<$listing>) {
  print "Result: $_";
}
close( $listing);

This code will properly reject shell commands:

% ./example.pl "dummy ; echo bad"
Argument contains unsanitary characters, stopped at ./example.pl line 8
% 

However, this code will also reject valid directories if they contain characters not in the white list regex.

Compliant Solution (Shell Avoidance)

This compliant solution again sanitizes the untrusted user input. However, it uses the multi-arg form of open().

my $file;
my $dir = $ARGV[0];
croak "Argument contains unsanitary characters, stopped" if ($dir =~ m|[^-A-Za-z0-9_/.~]|);
open( my $listing, "-|", "ls", "-F", $dir) or croak "error executing command: stopped";
while (<$listing>) {
  print "Result: $_";
}
close( $listing);

The perlfunc manpages states, regarding all but the first two arguments to open():

If there is only one scalar argument, the argument is checked for shell metacharacters, and if there are any, the entire argument is passed to the system's command shell for parsing (this is "/bin/sh -c" on Unix platforms, but varies on other platforms). If there are no shell metacharacters in the argument, it is split into words and passed directly to "execvp", which is more efficient.

So this form of open() is preferrable if your platform's shell might be set up incorrectly or maliciously.

Compliant Solution (Restricted Choice)

This compliant solution prevents command injection by passing only trusted strings to open(). While the user has control over which string is used, the user cannot provide string data directly to open().

my %choices = (BIN => "~/bin",
               LIB => "~/lib",
               SRC => "~/src");
my $choice = $choices{$ARGV[0]};
croak "Invalid argument, stopped" if (!defined $choice);
open( my $listing, "-|", "ls -F $choice") or croak "error executing command: stopped";
while (<$listing>) {
  print "Result: $_";
}
close( $listing);

This compliant solution hard-codes the directories that may be listed.

This solution can quickly become unmanageable if you have many available directories. A more scalable solution is to read all the permitted directories from an external file into a hash object, and the external file must be kept secure from untrusted users.

Compliant Solution (Avoid Interpreters)

When the task performed by executing a system command can be accomplished by some other means, it is almost always advisable to do so. This compliant solution uses the opendir(), readdir(), and closedir() subroutines to provide a directory listing, eliminating the possibility of command or argument injection attacks.

my $dir = $ARGV[0];
opendir( my $listing, $dir) or croak "error executing command: stopped";
while (readdir($listing)) {
  print "Result: $_\n";
}
closedir($listing);

Noncompliant Code Example (VU#583020)

US-CERT Vulnerability #583020  describes Perl code that invoked the system() builting function without sanitizing its argument:

sub do {
        shift;
        $do_call = "xmms -" . shift;
        system $do_call;
        return $do_call;
  }

An attacker who could control the arguments to the do() subroutine could cause the code to invoke arbitrary shell commands.

This code also violates DCL31-PL. Do not overload reserved keywords or subroutines.

Compliant Solution (VU#583020)

This code was mitigated by adding a regex to make sure that only a single character supplied by the user could be added to the $do_call variable before passing it to system().

sub do {
    shift;
    $command = shift;
    $command =~ /([\w])/;
    $command = $1;
    $do_call = "xmms -" . $command;
    system $do_call;
    return $do_call;
  }

This code still violates DCL31-PL. Do not overload reserved keywords or subroutines; it is shown here for historical accuracy.

Risk Assessment

Using deprecated or obsolete classes or methods in program code can lead to erroneous behavior.

Automated Detection

Related Guidelines

CERT C Secure Coding Standard: ENV04-C. Do not call system() if you do not need a command processor
CERT C++ Secure Coding Standard: ENV04-CPP. Do not call system() if you do not need a command processor
The CERT Oracle Secure Coding Standard for Java: IDS07-J. Do not pass untrusted, unsanitized data to the Runtime.exec() method

Bibliography

[Wall 2011] perlfunc
Vulnerability Note VU#583020: "XMMS Remote input validation error'