|  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #79330 shell_exec() silently truncates after a null byte
Submitted: 2020-03-01 19:19 UTC Modified: 2020-04-14 04:10 UTC
From: 64796c6e69 at gmail dot com Assigned: stas (profile)
Status: Closed Package: Program Execution
PHP Version: Irrelevant OS: any
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Bug Type:
From: 64796c6e69 at gmail dot com
New email:
PHP Version: OS:


 [2020-03-01 19:19 UTC] 64796c6e69 at gmail dot com
shell_exec() silently truncates anything after a null byte in the command it uses.

This was tested on PHP 7.3, but all supported PHP versions have this bug.

There's no issue if escapeshellarg() is used, but I thought it would be best to make this a security bug regardless. There may be some use case I haven't considered that creates a vulnerability. Checking for a null byte would also be consistent with exec() and other process functions.

The backtick operator has the same problem, but there is less risk of that causing a vulnerability.

Test script:
var_dump(shell_exec("echo before\0after"));

Expected result:
Warning: shell_exec(): NULL byte detected. Possible attack in shell_exec.php on line 2

Actual result:
string(7) "before


shell_exec.patch (last revision 2020-03-01 19:19 UTC by 64796c6e69 at gmail dot com)

Add a Patch

Pull Requests

Add a Pull Request


AllCommentsChangesGit/SVN commitsRelated reports
 [2020-03-21 17:02 UTC]
-Status: Open +Status: Verified -Assigned To: +Assigned To: stas
 [2020-03-21 17:02 UTC]
Instead of using the attached patch, I wonder whether we should
simply check for a valid path (like popen() does):

 ext/standard/exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/standard/exec.c b/ext/standard/exec.c
index b748e173ce..2d7961d908 100644
--- a/ext/standard/exec.c
+++ b/ext/standard/exec.c
@@ -528,7 +528,7 @@ PHP_FUNCTION(shell_exec)
 	php_stream *stream;
-		Z_PARAM_STRING(command, command_len)
+		Z_PARAM_PATH(command, command_len)
 #ifdef PHP_WIN32
 [2020-03-30 20:13 UTC] 64796c6e69 at gmail dot com
I think either would be fine. My patch causes shell_exec() to act the same as exec() for null bytes. I thought that made more sense only to make the error message more logical. "expects parameter 1 to be a valid path, string given" is a bit strange to see when the first argument is a command line. However, I'm also not opposed to that being the error. It would be more consistent with other functions.

Also, when I was seeing how different shell functions handle this, I found that proc_open() has the same problem. So, whichever implementation is chosen should be applied to that function as well.
 [2020-04-13 17:08 UTC]
Hmm, just noticed that this issue had already been reported as bug
 [2020-04-14 03:54 UTC]
I like the customized patch more, exec() argument is not really a path...
 [2020-04-14 04:10 UTC]
Automatic comment on behalf of stas
Log: Fix bug #79330 - make all execution modes consistent in rejecting \0
 [2020-04-14 04:10 UTC]
-Status: Verified +Status: Closed
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Jul 25 09:01:28 2024 UTC