php · binaryfire · Jul 21, 2026
diff --git a/package.xml b/package.xml
@@ -82,6 +82,9 @@
                 <file role="test" name="ssh2_sftp_001.phpt"/>
                 <file role="test" name="ssh2_sftp_002.phpt"/>
                 <file role="test" name="ssh2_shell.phpt"/>
+                <file role="test" name="ssh2_shell_arguments.phpt"/>
+                <file role="test" name="ssh2_shell_exec_environment_iteration.phpt"/>
+                <file role="test" name="ssh2_shell_exec_environment_redaction.phpt"/>
                 <file role="test" name="ssh2_skip.inc"/>
                 <file role="test" name="ssh2_stream_select.phpt"/>
                 <file role="test" name="ssh2_test.inc"/>

diff --git a/ssh2_fopen_wrappers.c b/ssh2_fopen_wrappers.c
@@ -540,29 +540,22 @@ static php_stream *php_ssh2_shell_open(LIBSSH2_SESSION *session, zend_resource *
 
 	if (environment) {
 		zend_string *key;
-		int key_type;
-		zend_ulong idx;
-
-		for(zend_hash_internal_pointer_reset(HASH_OF(environment));
-			(key_type = zend_hash_get_current_key(HASH_OF(environment), &key, &idx)) != HASH_KEY_NON_EXISTENT;
-			zend_hash_move_forward(HASH_OF(environment))) {
-			if (key_type == HASH_KEY_IS_STRING) {
-				zval *value;
-
-				if ((value = zend_hash_get_current_data(HASH_OF(environment))) != NULL) {
-					zval copyval = *value;
-
-					zval_copy_ctor(&copyval);
-					convert_to_string(&copyval);
-					if (libssh2_channel_setenv_ex(channel, key->val, key->len, Z_STRVAL(copyval), Z_STRLEN(copyval))) {
-						php_error_docref(NULL, E_WARNING, "Failed setting %s=%s on remote end", ZSTR_VAL(key), Z_STRVAL(copyval));
-					}
-					zval_dtor(&copyval);
+		zval *value;
+
+		ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(environment), key, value) {
+			if (key) {
+				zval copyval = *value;
+
+				zval_copy_ctor(&copyval);
+				convert_to_string(&copyval);
+				if (libssh2_channel_setenv_ex(channel, key->val, key->len, Z_STRVAL(copyval), Z_STRLEN(copyval))) {
+					php_error_docref(NULL, E_WARNING, "Failed setting environment variable %s on remote end", ZSTR_VAL(key));
 				}
+				zval_dtor(&copyval);
 			} else {
 				php_error_docref(NULL, E_NOTICE, "Skipping numeric index in environment array");
 			}
-		}
+		} ZEND_HASH_FOREACH_END();
 	}
 
 	if (type == PHP_SSH2_TERM_UNIT_CHARS) {
@@ -737,8 +730,8 @@ PHP_FUNCTION(ssh2_shell)
 	zend_long type = PHP_SSH2_DEFAULT_TERM_UNIT;
 	int argc = ZEND_NUM_ARGS();
 
-	if (argc == 5) {
-		php_error_docref(NULL, E_ERROR, "width specified without height parameter");
+	if (argc == 4) {
+		php_error_docref(NULL, E_WARNING, "width specified without height parameter");
 		RETURN_FALSE;
 	}
 
@@ -812,31 +805,23 @@ static php_stream *php_ssh2_exec_command(LIBSSH2_SESSION *session, zend_resource
 	}
 
 	if (environment) {
-		zend_string *key = NULL;
-		int key_type;
-		zend_ulong idx = 0;
-		HashPosition pos;
-
-		for(zend_hash_internal_pointer_reset_ex(HASH_OF(environment), &pos);
-			(key_type = zend_hash_get_current_key_ex(HASH_OF(environment), &key, &idx, &pos)) != HASH_KEY_NON_EXISTENT;
-			zend_hash_move_forward_ex(HASH_OF(environment), &pos)) {
-			if (key_type == HASH_KEY_IS_STRING) {
-				zval *value;
-
-				if ((value = zend_hash_get_current_data(HASH_OF(environment))) != NULL) {
-					zval copyval = *value;
-
-					zval_copy_ctor(&copyval);
-					convert_to_string(&copyval);
-					if (libssh2_channel_setenv_ex(channel, key->val, key->len, Z_STRVAL(copyval), Z_STRLEN(copyval))) {
-						php_error_docref(NULL, E_WARNING, "Failed setting %s=%s on remote end", ZSTR_VAL(key), Z_STRVAL(copyval));
-					}
-					zval_dtor(&copyval);
+		zend_string *key;
+		zval *value;
+
+		ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(environment), key, value) {
+			if (key) {
+				zval copyval = *value;
+
+				zval_copy_ctor(&copyval);
+				convert_to_string(&copyval);
+				if (libssh2_channel_setenv_ex(channel, key->val, key->len, Z_STRVAL(copyval), Z_STRLEN(copyval))) {
+					php_error_docref(NULL, E_WARNING, "Failed setting environment variable %s on remote end", ZSTR_VAL(key));
 				}
+				zval_dtor(&copyval);
 			} else {
 				php_error_docref(NULL, E_NOTICE, "Skipping numeric index in environment array");
 			}
-		}
+		} ZEND_HASH_FOREACH_END();
 	}
 
 	if (term) {

diff --git a/tests/ssh2_shell_arguments.phpt b/tests/ssh2_shell_arguments.phpt
@@ -0,0 +1,34 @@
+--TEST--
+ssh2_shell() validates paired terminal dimensions
+--SKIPIF--
+<?php
+require 'ssh2_skip.inc';
+ssh2t_needs_auth();
+?>
+--FILE--
+<?php
+require 'ssh2_test.inc';
+
+$session = ssh2_connect(TEST_SSH2_HOSTNAME, TEST_SSH2_PORT);
+
+if ($session === false || !ssh2t_auth($session)) {
+    throw new RuntimeException('Failed to connect to the SSH fixture.');
+}
+
+$shell = ssh2_shell($session, 'xterm', [], 80, 24);
+
+var_dump(is_resource($shell));
+
+if (is_resource($shell)) {
+    fclose($shell);
+}
+
+var_dump(ssh2_shell($session, 'xterm', [], 80));
+
+ssh2_disconnect($session);
+?>
+--EXPECTF--
+bool(true)
+
+Warning: ssh2_shell(): width specified without height parameter in %s on line %d
+bool(false)
diff --git a/tests/ssh2_shell_exec_environment_iteration.phpt b/tests/ssh2_shell_exec_environment_iteration.phpt
@@ -0,0 +1,62 @@
+--TEST--
+Shell and exec environment arrays are traversed safely
+--SKIPIF--
+<?php
+require 'ssh2_skip.inc';
+ssh2t_needs_auth();
+?>
+--FILE--
+<?php
+require 'ssh2_test.inc';
+
+final class EnvironmentValue
+{
+    public static array $converted = [];
+
+    public function __construct(private string $value)
+    {
+    }
+
+    public function __toString(): string
+    {
+        self::$converted[] = $this->value;
+
+        return $this->value;
+    }
+}
+
+$session = ssh2_connect(TEST_SSH2_HOSTNAME, TEST_SSH2_PORT);
+
+if ($session === false || !ssh2t_auth($session)) {
+    throw new RuntimeException('Failed to connect to the SSH fixture.');
+}
+
+$shell = ssh2_shell($session, 'xterm', [], 80, 24);
+
+var_dump(is_resource($shell));
+
+if (is_resource($shell)) {
+    fclose($shell);
+}
+
+$stream = @ssh2_exec($session, 'true', null, [
+    'PHP_SSH2_TEST_A' => new EnvironmentValue('alpha'),
+    'PHP_SSH2_TEST_B' => new EnvironmentValue('bravo'),
+]);
+
+var_dump(EnvironmentValue::$converted);
+
+if (is_resource($stream)) {
+    fclose($stream);
+}
+
+ssh2_disconnect($session);
+?>
+--EXPECT--
+bool(true)
+array(2) {
+  [0]=>
+  string(5) "alpha"
+  [1]=>
+  string(5) "bravo"
+}
diff --git a/tests/ssh2_shell_exec_environment_redaction.phpt b/tests/ssh2_shell_exec_environment_redaction.phpt
@@ -0,0 +1,65 @@
+--TEST--
+Shell and exec environment warnings do not expose values
+--SKIPIF--
+<?php
+require 'ssh2_skip.inc';
+ssh2t_needs_auth();
+?>
+--FILE--
+<?php
+require 'ssh2_test.inc';
+
+$session = ssh2_connect(TEST_SSH2_HOSTNAME, TEST_SSH2_PORT);
+
+if ($session === false || !ssh2t_auth($session)) {
+    throw new RuntimeException('Failed to connect to the SSH fixture.');
+}
+
+$secret = 'secret-value-must-not-appear';
+$warnings = [];
+
+set_error_handler(static function (int $severity, string $message) use (&$warnings): bool {
+    if ($severity !== E_WARNING || !str_contains($message, 'Failed setting environment variable')) {
+        return false;
+    }
+
+    $warnings[] = $message;
+
+    return true;
+});
+
+try {
+    $environment = ['PHP_SSH2_REDACTION_TEST' => $secret];
+    $shell = ssh2_shell($session, 'xterm', $environment, 80, 24);
+    $stream = ssh2_exec($session, 'true', null, $environment);
+} finally {
+    restore_error_handler();
+}
+
+var_dump(is_resource($shell));
+var_dump(is_resource($stream));
+
+foreach ($warnings as $warning) {
+    echo $warning, PHP_EOL;
+}
+
+var_dump(count($warnings));
+var_dump(str_contains(implode("\n", $warnings), $secret));
+
+if (is_resource($shell)) {
+    fclose($shell);
+}
+
+if (is_resource($stream)) {
+    fclose($stream);
+}
+
+ssh2_disconnect($session);
+?>
+--EXPECT--
+bool(true)
+bool(true)
+ssh2_shell(): Failed setting environment variable PHP_SSH2_REDACTION_TEST on remote end
+ssh2_exec(): Failed setting environment variable PHP_SSH2_REDACTION_TEST on remote end
+int(2)
+bool(false)