diff --git a/package.xml b/package.xml index 07c0427..2faf69f 100644 --- a/package.xml +++ b/package.xml @@ -82,6 +82,9 @@ + + + diff --git a/ssh2_fopen_wrappers.c b/ssh2_fopen_wrappers.c index f1b3ce1..76585ec 100644 --- 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(©val); - convert_to_string(©val); - 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(©val); + zval *value; + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(environment), key, value) { + if (key) { + zval copyval = *value; + + zval_copy_ctor(©val); + convert_to_string(©val); + 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(©val); } 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(©val); - convert_to_string(©val); - 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(©val); + 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(©val); + convert_to_string(©val); + 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(©val); } 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 new file mode 100644 index 0000000..a31312c --- /dev/null +++ b/tests/ssh2_shell_arguments.phpt @@ -0,0 +1,34 @@ +--TEST-- +ssh2_shell() validates paired terminal dimensions +--SKIPIF-- + +--FILE-- + +--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 new file mode 100644 index 0000000..d72f627 --- /dev/null +++ b/tests/ssh2_shell_exec_environment_iteration.phpt @@ -0,0 +1,62 @@ +--TEST-- +Shell and exec environment arrays are traversed safely +--SKIPIF-- + +--FILE-- +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 new file mode 100644 index 0000000..1fb57b2 --- /dev/null +++ b/tests/ssh2_shell_exec_environment_redaction.phpt @@ -0,0 +1,65 @@ +--TEST-- +Shell and exec environment warnings do not expose values +--SKIPIF-- + +--FILE-- + $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)