add nullbyte detection to shell_exec,exec,system,passthru

Summary: php compat: php/php-src@a8722f5 Reviewed By: @markw65 Differential Revision: D2082131
facebook · May 22, 2015 · f67f6c4 · f67f6c4
1 parent a469890
commit f67f6c4
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 12 deletions.
diff --git a/hphp/runtime/ext/process/ext_process.cpp b/hphp/runtime/ext/process/ext_process.cpp
@@ -516,9 +516,11 @@ int64_t HHVM_FUNCTION(pcntl_wtermsig,
 
 #define EXEC_INPUT_BUF 4096
 
-class ShellExecContext {
+namespace {
+
+class ShellExecContext final {
 public:
-  ShellExecContext() : m_proc(NULL) {
+  ShellExecContext() {
     m_sig_handler = signal(SIGCHLD, SIG_DFL);
   }
 
@@ -531,33 +533,40 @@ class ShellExecContext {
     }
   }
 
-  FILE *exec(const char *cmd) {
-    assert(m_proc == NULL);
+  FILE *exec(const String& cmd_string) {
+    assert(m_proc == nullptr);
+    const auto cmd = cmd_string.c_str();
     if (RuntimeOption::WhitelistExec && !check_cmd(cmd)) {
-      return NULL;
+      return nullptr;
+    }
+    if (strlen(cmd) != cmd_string.size()) {
+      raise_warning("NULL byte detected. Possible attack");
+      return nullptr;
     }
     m_proc = LightProcess::popen(cmd, "r", g_context->getCwd().data());
-    if (m_proc == NULL) {
+    if (m_proc == nullptr) {
       raise_warning("Unable to execute '%s'", cmd);
     }
     return m_proc;
   }
 
   int exit() {
     int status = LightProcess::pclose(m_proc);
-    m_proc = NULL;
+    m_proc = nullptr;
     return status;
   }
 
 private:
   void (*m_sig_handler)(int);
-  FILE *m_proc;
+  FILE *m_proc{nullptr};
 };
 
+}
+
 Variant HHVM_FUNCTION(shell_exec,
                       const String& cmd) {
   ShellExecContext ctx;
-  FILE *fp = ctx.exec(cmd.c_str());
+  FILE *fp = ctx.exec(cmd);
   if (!fp) return init_null();
   StringBuffer sbuf;
   sbuf.read(fp);
@@ -574,7 +583,7 @@ String HHVM_FUNCTION(exec,
                      VRefParam output /* = null */,
                      VRefParam return_var /* = null */) {
   ShellExecContext ctx;
-  FILE *fp = ctx.exec(command.c_str());
+  FILE *fp = ctx.exec(command);
   if (!fp) return empty_string();
   StringBuffer sbuf;
   sbuf.read(fp);
@@ -605,7 +614,7 @@ void HHVM_FUNCTION(passthru,
                    const String& command,
                    VRefParam return_var /* = null */) {
   ShellExecContext ctx;
-  FILE *fp = ctx.exec(command.c_str());
+  FILE *fp = ctx.exec(command);
   if (!fp) return;
 
   char buffer[1024];
@@ -625,7 +634,7 @@ String HHVM_FUNCTION(system,
                      const String& command,
                      VRefParam return_var /* = null */) {
   ShellExecContext ctx;
-  FILE *fp = ctx.exec(command.c_str());
+  FILE *fp = ctx.exec(command);
   if (!fp) return empty_string();
   StringBuffer sbuf;
   if (fp) {

diff --git a/hphp/test/slow/ext_process/lwp.php b/hphp/test/slow/ext_process/lwp.php
@@ -105,3 +105,10 @@ function VERIFY($x) { VS($x != false, true); }
 VS(escapeshellarg("\""), "'\"'");
 
 VS(escapeshellcmd("perl \""), "perl \\\"");
+
+$nullbyte = "echo abc\n\0command";
+VS(passthru($nullbyte), null);
+VS(system($nullbyte), "");
+VS(exec($nullbyte, $nullbyteout), "");
+VS($nullbyteout, null);
+VS(shell_exec($nullbyte), null);
diff --git a/hphp/test/slow/ext_process/lwp.php.expect → hphp/test/slow/ext_process/lwp.php.expectf b/hphp/test/slow/ext_process/lwp.php.expect → hphp/test/slow/ext_process/lwp.php.expectf
@@ -37,3 +37,16 @@ bool(true)
 bool(true)
 bool(true)
 bool(true)
+
+Warning: NULL byte detected. Possible attack in %s/lwp.php on line 110
+bool(true)
+
+Warning: NULL byte detected. Possible attack in %s/lwp.php on line 111
+bool(true)
+
+Warning: NULL byte detected. Possible attack in %s/lwp.php on line 112
+bool(true)
+bool(true)
+
+Warning: NULL byte detected. Possible attack in %s/lwp.php on line 114
+bool(true)