php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #61706 escapeshellarg behaves inconsistently depending on shell
Submitted: 2012-04-12 22:22 UTC Modified: -
Votes:4
Avg. Score:4.0 ± 1.0
Reproduced:2 of 3 (66.7%)
Same Version:1 (50.0%)
Same OS:2 (100.0%)
From: phpbugs at personal dot formauri dot es Assigned:
Status: Open Package: Program Execution
PHP Version: 5.4Git-2012-04-12 (Git) OS: Linux, Unix, maybe OSX, NOT msw
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: phpbugs at personal dot formauri dot es
New email:
PHP Version: OS:

 

 [2012-04-12 22:22 UTC] phpbugs at personal dot formauri dot es
Description:
------------
Depending on the shell, for shell internal commands the backslashes within single quotes are interpreted as escapes or are used verbatim. For example, in bash and in busybox:

$ echo '\\'
\\

But in dash:

$ echo '\\'
\

dash is frequently set as the default /bin/sh so this is a problem. More so since some programs need to get their input from stdin and therefore they need the use of 'echo' for input not coming from a file or being input from the console.

To work around the backslash inconsistency among shells, backslashes should receive special treatment as quotes do, e.g. translate \ to '\\'.

I was tempted of sending this as a security issue, but the scenarios where security could be in risk are too improbable for it to be a serious security concern.

Ideally though, no unnecessary quotes should be used in the output string, e.g. escapeshellarg should convert '''abc\\'\ into \'\'\''abc'\\\\\'\\. Currently it converts '''abc\\'\ into ''\'''\'''\''abc\\'\''\' which exhibits the bug and is unnecessarily large.

For backwards compatibility, maybe an extra argument should be added to also quote backslashes and use a new method of quoting.

Here is a PHP function that implements the suggestions here, using strspn and strcspn to grab the longest spans that it can "eat" at a time of each kind (characters to escape / characters not to escape): http://www.formauri.es/personal/pgimeno/temp/sh_escape.phps (includes test suite).


Test script:
---------------
<?php
  $backslash = "\\";
  system('echo ' . escapeshellarg($backslash . $backslash));
?>


Expected result:
----------------
No matter the shell:
\\


Actual result:
--------------
If your /bin/sh is dash:
\
If your /bin/sh is busybox:
\\
Other shells: ??


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-04-13 00:51 UTC] zhanglijiu at gmail dot com
My result is \\
my system is Mac OS
SHould be bash
 [2013-04-24 11:39 UTC] phpbugs at personal dot formauri dot es
For extra background, see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=550399

Unlike what I thought at first, it seems to affect the shell's built-in echo command specifically. As noted in that bug, it also affects shells other than dash, including posh and mksh. I've had the problem with zsh as well. And as noted in that bug, dash is the default shell in many systems, and also in Ubuntu (see https://bugs.launchpad.net/ubuntu/+source/dash/+bug/259671 ). Debian also offers to install posh as the default /bin/sh. The consensus seems to be that that is not a bug in the shell, because the result of using backslashes in the shell's builtin echo is implementation-defined, and therefore it's PHP's responsibility to escape them properly, e.g. in the suggested way.

On a different topic, one advantage of using the method of switching mode depending on the runs of should-be-escaped/should-not-be-escaped characters, as in the PHP example function shown above, is that the temporary storage requirement is reduced from 4n+2 as is now, or ~4 times the length, to ceil(5n/2), or ~2.5 times the length. That's because the worst case for the current behavior is a sequence of single-quotes which is written as ''\'''\'''\''...'\''' and the worst case for the proposed behaviour is alternating escaped/non-escaped characters as in 'x'\\'x'\\'x'\\...'x', therefore every 2 characters are turned into 5 with possibly an extra character at the end.
 [2013-04-24 13:20 UTC] phpbugs at personal dot formauri dot es
Patch here: http://www.formauri.es/personal/pgimeno/temp/escapeshellarg-bug-61706.patch
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sun Nov 24 03:01:32 2024 UTC