Bug #60116 [Asn]: escapeshellcmd() cannot escape the chars which causes shell injection.
Edit report at https://bugs.php.net/bug.php?id=60116&edit=1
ID: 60116
Updated by: lbarnaud@php.net
Reported by: hirokawa@php.net
Summary: escapeshellcmd() cannot escape the chars which
causes shell injection.
Status: Assigned
Type: Bug
Package: Filter related
Operating System: Ubuntu Linux
PHP Version: trunk-SVN-2011-10-23 (SVN)
Assigned To: hirokawa
Block user comment: N
Private report: N
New Comment:
> The default behavier which not escaped paired quotes is still dangerous even if the
> single-quotes is used.
Yes, I was speaking of both single quotes and double quote: Don't enclose the escaped string in
quotes at all :)
This is a bit puzzling because all escaping function like mysql_escape_string expect the user to
enclose the string in quotes. But escapeshellcmd and escapeshellarg don't.
It's like htmlspecialchars: it just removes the special meaning of special characters.
> But, generally, escapeshellcmd() is used to escape the user input
It shouldn't be the case. escapeshellcmd escapes all control characters from a string, which
avoids command injection, redirection, etc but doesn't prevent argument injection (it
doesn't escape spaces).
Previous Comments:
------------------------------------------------------------------------
[2011-11-10 22:49:42] hirokawa@php.net
The default behavier which not escaped paired quotes is still dangerous even if
the single-quotes is used.
$_GET['key'] = ":' '/etc/hosts";
$key = escapeshellcmd($_GET['key']);
$cmd = "grep '$key' /var/data/*"; // <- single quote
system($cmd); // output: grep ':' '/etc/hosts' /var/data/*
You are right, escapeshellarg() is better than escapeshellcmd() in this case.
But, generally, escapeshellcmd() is used to escape the user input
(GET/POST/Cookie), the default behavior (paired quotes are not escaped) is
not recommended.
------------------------------------------------------------------------
[2011-11-10 15:18:49] lbarnaud@php.net
The example at http://docs.php.net/manual/en/function.escapeshellcmd.php
is wrong. It is enclosing an escaped argument in double quotes, but the escapeshellcmd function
doesn't expect this.
As a result the second command in the example is unsafe.
IMO the second command in the example should be removed and replaced by a warning telling to use
escapeshellarg instead (because escapeshellcmd doesn't escape spaces and an argument escaped by
escapeshellcmd may be interpreted as multiple arguments by the shell).
------------------------------------------------------------------------
[2011-11-10 15:09:03] lbarnaud@php.net
Hi,
It seems that you are not using escapeshellcmd() correctly, and that's why it's unsafe in
the way you are using it.
You are enclosing escapeshellcmd's output in double quotes.
However escapeshellcmd() and escapeshellarg() do not work like mysql_real_escape_string() for
example, and you must *not* enclose the string in quotes yourself. (The example in the documentation
is wrong.)
When you don't do it it's perfectly fine:
echo escapeshellcmd('foo" "bar');
Result: foo" "bar // the quotes don't allow to inject a command.
echo escapeshellcmd('foo"bar')
Result: foo\"bar // This time the quote is escaped since it's not paired. Again, injecting
a command is not possible.
Also, I believe that escapeshell*arg*() should be used instead or escapeshell*cmd*() when escaping
an argument:
$cmd = sprintf('grep %s /var/data/*', escapeshellarg($_GET['key']));
(escapeshellcmd() won't escape spaces and would allow to inject an additional argument;
escapeshellarg() encloses the whole argument in single quotes and ensures that it's treated as
a single argument)
------------------------------------------------------------------------
[2011-11-10 14:19:09] hirokawa@php.net
Automatic comment from SVN on behalf of hirokawa
Revision: http://svn.php.net/viewvc/?view=revision&revision=318996
Log: MFH: fixed bug #60116 (escapeshellcmd() cannot escape the characters which cause shell command
injection).
------------------------------------------------------------------------
[2011-10-30 05:57:22] hirokawa@php.net
Automatic comment from SVN on behalf of hirokawa
Revision: http://svn.php.net/viewvc/?view=revision&revision=318568
Log: added a test script for bug60116 and fixed behabior of ESCAPE_CMD_END.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=60116
--
Edit this bug report at https://bugs.php.net/bug.php?id=60116&edit=1
Thread (10 messages)