escapeshellcmd warning

Question

In the php documentation pages for escapeshellcmd there is a warning:

escapeshellcmd() should be used on the whole command string, and it still allows the attacker to pass arbitrary number of arguments. For escaping a single argument escapeshellarg() should be used instead.

What am I to understand from this:

1. Should I always escape the whole command string using escapeshellcmd, including arguments that have already been escaped with escpaeshellarg?
2. Should I only escape command items that are not parameters (The only logical thing to do if you ask me)?
3. Should I just ignore this dubious warning that creates even more confusion about how these 2 functions compliment each other?

Thank you, Cosmin

Answer 1

In short

• escapeshellarg: is used to enclose an argument with single quotes and it escapes the quotes within the argument.
• escapeshellcmd: is used to escape the shell metacharacters i.e. <,>,| etc

Assuming that your version of php relies on bash for command execution, we know from the bash manual,

Enclosing characters in single quotes preserves the literal value of each character within the quotes. A single quote may not occur between single quotes, even when preceded by a backslash.

Therefore, we can come to the conclusion that the following way of creating a command should be sufficient:

$c = escapeshellcmd( $cmd ) . ' ' . escapeshellarg( $arg1 );

However, this can give you problems if you need to pass $c to one of the following functions: exec, system, passthru etc. Empirical analysis has shown that these functions can evaluate some of the characters. As an example try this:

$cmd = 'echo';
$arg = 'TEST\0ING'; // Since we are using single quotes, \0 shouldn't be evaluated as a null byte char
$c = escapeshellcmd( $cmd ) . ' ' . escapeshellarg( $arg1 ); // echo 'TEST\0ING'
exec( $c . ' > output.txt'); // writes TEST without the ING to output.txt

The exec function evaluates the '\0' as a null byte character and therefore the output.txt will only contain the data upto '\0'. This has been tested on Ubuntu running PHP 5.4.6. In my opinion, this is a bug because the literal meaning of the data is lost. So, it is relatively safer to use escapeshellcmd on the complete string as it also escapes the backslash '\' which is a shell metacharacter:

$c = escapeshellcmd( $cmd . ' ' . escapeshellarg( $arg1 ) );
exec( $c . ' > output.txt'); // writes TEST\0ING to output.txt

The downside of this approach is that other metacharacters will also be escaped and will have to be unescaped by the program ($cmd) to which the arguments are being passed to.

Note that we didn't use escapeshellcmd on the complete command string. We left out the ' > output.txt' part so that the '>' doesn't get escaped.

Conclusions:

• Try to avoid passing strings as command line arguments. Instead, write data to files and read from files. This way you can be sure that the data being passed stays unmodified.

• If you absolutely need to pass the strings as command line arguments, always make sure that they are alphanumeric.

• If you absolutely need to pass strings as command line arguments, using escapeshellcmd on the complete string is relatively safer.

Answer 2

So I think escapeshellcmd prevents more than one command from being run.

escapeshellcmd() escapes any characters in a string that might be used to trick a shell command into executing arbitrary commands.

escapeshellarg is to ensure that a user provided argument is just one argument.

escapeshellarg() adds single quotes around a string and quotes/escapes any existing single quotes allowing you to pass a string directly to a shell function and having it be treated as a single safe argument.

So essentially, you should only need to use escapeshellarg on each user provided argument if you (your program) are defining the command, but both if the user dictates the command (which is already pretty iffy unless you have really solid server permissions set up) and arguments.

DISCLAIMER: This is just my interpretation of the documentation.

EDIT:

I agree that the documentation is extremely confusing here. I think the warning is implying that wrapping this around a whole command would not prevent a user from using something like this an argument

actualArg -someEvilFlag

or

actualArg -someEvilFlag evilFlagArgument