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

4 files changed, 48 insertions, 35 deletions

diff --git a/NEWS b/NEWS
index fdb232695f..2b2c4c4454 100644
--- 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 700d4e862c..6969faebe9 100644
--- 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
index 0000000000..05c25e6f1e
--- /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 2c08ce87b7..f0a3cf83e8 100644
--- a/ext/standard/tests/streams/bug60602.phpt
+++ b/ext/standard/tests/streams/bug60602.phpt
@@ -49,8 +49,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)
 bool(true)