Fixed bug #72306 (Heap overflow through proc_open and $env parameter)

diff --git a/NEWS b/NEWS
index fdb232695fb109904f22e4b5527e5d9f9ada00ad..2b2c4c445424687aa7aaebc85f6c0118cf597b22 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,9 @@ PHP                                                                        NEWS
 ?? ??? 2016 PHP 7.0.9
 
 
+- Standard:
+  . Fixed bug #72306 (Heap overflow through proc_open and $env parameter).
+    (Laruence)
 
 23 Jun 2016 PHP 7.0.8
 

diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c
index 700d4e862c5a6c721488bb32405c5cca13489a81..6969faebe97bff90c6eb7f97a5c84e7b3523697b 100644 (file)
--- a/ext/standard/proc_open.c
+++ b/ext/standard/proc_open.c
@@ -77,13 +77,13 @@ static php_process_env_t _php_array_to_envp(zval *environment, int is_persistent
 {
        zval *element;
        php_process_env_t env;
-       zend_string *string_key;
+       zend_string *key, *str;
 #ifndef PHP_WIN32
        char **ep;
 #endif
        char *p;
-       size_t cnt, l, sizeenv=0;
-       HashTable *target_hash;
+       size_t cnt, l, sizeenv = 0;
+       HashTable *env_hash;
 
        memset(&env, 0, sizeof(env));
 
@@ -101,28 +101,25 @@ static php_process_env_t _php_array_to_envp(zval *environment, int is_persistent
                return env;
        }
 
-       target_hash = Z_ARRVAL_P(environment);
-       if (!target_hash) {
-               return env;
-       }
+       ALLOC_HASHTABLE(env_hash);
+       zend_hash_init(env_hash, cnt, NULL, NULL, 0);
 
        /* first, we have to get the size of all the elements in the hash */
-       ZEND_HASH_FOREACH_STR_KEY_VAL(target_hash, string_key, element) {
-               zend_string *str = zval_get_string(element);
-               size_t el_len = ZSTR_LEN(str);
-               zend_string_release(str);
+       ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(environment), key, element) {
+               str = zval_get_string(element);
 
-               if (el_len == 0) {
+               if (ZSTR_LEN(str) == 0) {
+                       zend_string_release(str);
                        continue;
                }
 
-               sizeenv += el_len + 1;
+               sizeenv += ZSTR_LEN(str) + 1;
 
-               if (string_key) {
-                       if (ZSTR_LEN(string_key) == 0) {
-                               continue;
-                       }
-                       sizeenv += ZSTR_LEN(string_key) + 1;
+               if (key && ZSTR_LEN(key)) {
+                       sizeenv += ZSTR_LEN(key) + 1;
+                       zend_hash_add_ptr(env_hash, key, str);
+               } else {
+                       zend_hash_next_index_insert_ptr(env_hash, str);
                }
        } ZEND_HASH_FOREACH_END();
 
@@ -131,20 +128,10 @@ static php_process_env_t _php_array_to_envp(zval *environment, int is_persistent
 #endif
        p = env.envp = (char *) pecalloc(sizeenv + 4, 1, is_persistent);
 
-       ZEND_HASH_FOREACH_STR_KEY_VAL(target_hash, string_key, element) {
-               zend_string *str = zval_get_string(element);
-
-               if (ZSTR_LEN(str) == 0) {
-                       goto next_element;
-               }
-
-               if (string_key) {
-                       if (ZSTR_LEN(string_key) == 0) {
-                               goto next_element;
-                       }
-
-                       l = ZSTR_LEN(string_key) + ZSTR_LEN(str) + 2;
-                       memcpy(p, ZSTR_VAL(string_key), ZSTR_LEN(string_key));
+       ZEND_HASH_FOREACH_STR_KEY_PTR(env_hash, key, str) {
+               if (key) {
+                       l = ZSTR_LEN(key) + ZSTR_LEN(str) + 2;
+                       memcpy(p, ZSTR_VAL(key), ZSTR_LEN(key));
                        strncat(p, "=", 1);
                        strncat(p, ZSTR_VAL(str), ZSTR_LEN(str));
 
@@ -161,12 +148,14 @@ static php_process_env_t _php_array_to_envp(zval *environment, int is_persistent
 #endif
                        p += ZSTR_LEN(str) + 1;
                }
-next_element:
                zend_string_release(str);
        } ZEND_HASH_FOREACH_END();
 
        assert((uint)(p - env.envp) <= sizeenv);
 
+       zend_hash_destroy(env_hash);
+       FREE_HASHTABLE(env_hash);
+
        return env;
 }
 /* }}} */

diff --git a/ext/standard/tests/general_functions/bug72306.phpt b/ext/standard/tests/general_functions/bug72306.phpt
new file mode 100644 (file)
index 0000000..05c25e6
--- /dev/null
+++ b/ext/standard/tests/general_functions/bug72306.phpt
@@ -0,0 +1,23 @@
+--TEST--
+Bug #72306 (Heap overflow through proc_open and $env parameter)
+--FILE--
+<?php
+class moo {
+       function __construct() { $this->a = 0; }
+       function __toString() { return $this->a++ ? str_repeat("a", 0x8000) : "a"; }
+}
+
+$env = array('some_option' => new moo());
+$pipes = array();
+$description = array(
+   0 => array("pipe", "r"),
+   1 => array("pipe", "w"),
+   2 => array("pipe", "r")
+);
+
+$process = proc_open('nothing', $description, $pipes, NULL, $env);
+
+?>
+okey
+--EXPECT--
+okey

diff --git a/ext/standard/tests/streams/bug60602.phpt b/ext/standard/tests/streams/bug60602.phpt
index 2c08ce87b736bf8c651e792a7e6c715c46d56676..f0a3cf83e870c5bdddf5936575925c3455f776dd 100644 (file)
--- a/ext/standard/tests/streams/bug60602.phpt
+++ b/ext/standard/tests/streams/bug60602.phpt
@@ -48,8 +48,6 @@ if (is_resource($p)) {
 ?>
 ==DONE==
 --EXPECTF--
-Notice: Array to string conversion in %s on line %d
-
 Notice: Array to string conversion in %s on line %d
 int(%d)
 int(0)