From 23f5b6debdd24cc1caefd3bb8cd6da96a88abe9a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 24 Nov 2016 22:22:38 +0100 Subject: [PATCH 1/5] [ticket/14875] Add method for untrimmed input to ajax iohandler Due to the pre-encoded input and the escaping of the input, the string has to be decoded twice for the password. PHPBB3-14875 --- .../install/helper/iohandler/ajax_iohandler.php | 16 ++++++++++++++++ .../obtain_data/task/obtain_database_data.php | 11 ++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php index c168d26425..591a19b7c1 100644 --- a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php +++ b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php @@ -120,6 +120,22 @@ class ajax_iohandler extends iohandler_base return $this->request->variable($name, $default, $multibyte); } + /** + * Returns untrimmed input variable + * + * @param string $name Name of the input variable to obtain + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * @param bool $multibyte If $default is a string this paramater has to be true if the variable may contain any UTF-8 characters + * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks + * + * @return mixed Value of the untrimmed input variable + */ + public function get_untrimmed_input($name, $default, $multibyte = false) + { + return $this->request->untrimmed_variable($name, $default, $multibyte); + } + /** * {@inheritdoc} */ diff --git a/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php b/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php index ce720dbf76..9019cf4332 100644 --- a/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php +++ b/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php @@ -79,10 +79,19 @@ class obtain_database_data extends \phpbb\install\task_base implements \phpbb\in $dbhost = $this->io_handler->get_input('dbhost', '', true); $dbport = $this->io_handler->get_input('dbport', ''); $dbuser = $this->io_handler->get_input('dbuser', ''); - $dbpasswd = $this->io_handler->get_input('dbpasswd', '', true); $dbname = $this->io_handler->get_input('dbname', ''); $table_prefix = $this->io_handler->get_input('table_prefix', ''); + // Need to get untrimmed password when using ajax IO handler + if ($this->io_handler instanceof \phpbb\install\helper\iohandler\ajax_iohandler) + { + $dbpasswd = htmlspecialchars_decode(htmlspecialchars_decode($this->io_handler->get_untrimmed_input('dbpasswd', '', true))); + } + else + { + $dbpasswd = $this->io_handler->get_input('dbpasswd', '', true); + } + // Check database data $user_data_vaild = $this->check_database_data($dbms, $dbhost, $dbport, $dbuser, $dbpasswd, $dbname, $table_prefix); From 9aa017d0f7ce13a11114cbae24b694e935931342 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 25 Nov 2016 22:15:13 +0100 Subject: [PATCH 2/5] [ticket/14875] Add method for raw input to request and add to installer A method for retrieving raw input has been added to the request class. This will be used in the installer to retrieve the datatabase password while also allowing utf8 characters. Not escaping the input is ok in this case as it won't be put anywhere in this raw form and only be used to populate the entry for the password field in config.php. PHPBB3-14875 --- .../helper/iohandler/ajax_iohandler.php | 20 ++---- .../helper/iohandler/cli_iohandler.php | 14 +++++ .../helper/iohandler/iohandler_interface.php | 11 ++++ .../obtain_data/task/obtain_database_data.php | 11 +--- phpBB/phpbb/request/request.php | 62 +++++++++++++++++++ 5 files changed, 94 insertions(+), 24 deletions(-) diff --git a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php index 591a19b7c1..2db6750f3f 100644 --- a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php +++ b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php @@ -27,7 +27,7 @@ class ajax_iohandler extends iohandler_base protected $path_helper; /** - * @var \phpbb\request\request_interface + * @var \phpbb\request\request */ protected $request; @@ -90,12 +90,12 @@ class ajax_iohandler extends iohandler_base * Constructor * * @param path_helper $path_helper - * @param \phpbb\request\request_interface $request HTTP request interface + * @param \phpbb\request\request $request HTTP request interface * @param \phpbb\template\template $template Template engine * @param router $router Router * @param string $root_path Path to phpBB's root */ - public function __construct(path_helper $path_helper, \phpbb\request\request_interface $request, \phpbb\template\template $template, router $router, $root_path) + public function __construct(path_helper $path_helper, \phpbb\request\request $request, \phpbb\template\template $template, router $router, $root_path) { $this->path_helper = $path_helper; $this->request = $request; @@ -121,19 +121,11 @@ class ajax_iohandler extends iohandler_base } /** - * Returns untrimmed input variable - * - * @param string $name Name of the input variable to obtain - * @param mixed $default A default value that is returned if the variable was not set. - * This function will always return a value of the same type as the default. - * @param bool $multibyte If $default is a string this paramater has to be true if the variable may contain any UTF-8 characters - * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks - * - * @return mixed Value of the untrimmed input variable + * {@inheritdoc} */ - public function get_untrimmed_input($name, $default, $multibyte = false) + public function get_raw_input($name, $default) { - return $this->request->untrimmed_variable($name, $default, $multibyte); + return $this->request->raw_variable($name, $default); } /** diff --git a/phpBB/phpbb/install/helper/iohandler/cli_iohandler.php b/phpBB/phpbb/install/helper/iohandler/cli_iohandler.php index 196cdcdaab..4117a3dfd3 100644 --- a/phpBB/phpbb/install/helper/iohandler/cli_iohandler.php +++ b/phpBB/phpbb/install/helper/iohandler/cli_iohandler.php @@ -74,6 +74,20 @@ class cli_iohandler extends iohandler_base return $result; } + /** + * {@inheritdoc} + */ + public function get_raw_input($name, $default) + { + return $this->get_input($name, $default, true); + } + + /** + * Set input variable + * + * @param string $name Name of input variable + * @param mixed $value Value of input variable + */ public function set_input($name, $value) { $this->input_values[$name] = $value; diff --git a/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php b/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php index f22f33d9cb..f0e0e99bbb 100644 --- a/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php +++ b/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php @@ -38,6 +38,17 @@ interface iohandler_interface */ public function get_input($name, $default, $multibyte = false); + /** + * Returns raw input variable + * + * @param string $name Name of the input variable to obtain + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * + * @return mixed Value of the raw input variable + */ + public function get_raw_input($name, $default); + /** * Returns server variable * diff --git a/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php b/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php index 9019cf4332..dc7b060746 100644 --- a/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php +++ b/phpBB/phpbb/install/module/obtain_data/task/obtain_database_data.php @@ -79,19 +79,10 @@ class obtain_database_data extends \phpbb\install\task_base implements \phpbb\in $dbhost = $this->io_handler->get_input('dbhost', '', true); $dbport = $this->io_handler->get_input('dbport', ''); $dbuser = $this->io_handler->get_input('dbuser', ''); + $dbpasswd = $this->io_handler->get_raw_input('dbpasswd', ''); $dbname = $this->io_handler->get_input('dbname', ''); $table_prefix = $this->io_handler->get_input('table_prefix', ''); - // Need to get untrimmed password when using ajax IO handler - if ($this->io_handler instanceof \phpbb\install\helper\iohandler\ajax_iohandler) - { - $dbpasswd = htmlspecialchars_decode(htmlspecialchars_decode($this->io_handler->get_untrimmed_input('dbpasswd', '', true))); - } - else - { - $dbpasswd = $this->io_handler->get_input('dbpasswd', '', true); - } - // Check database data $user_data_vaild = $this->check_database_data($dbms, $dbhost, $dbport, $dbuser, $dbpasswd, $dbname, $table_prefix); diff --git a/phpBB/phpbb/request/request.php b/phpBB/phpbb/request/request.php index 4cac6fbaea..318d9f66f9 100644 --- a/phpBB/phpbb/request/request.php +++ b/phpBB/phpbb/request/request.php @@ -224,6 +224,68 @@ class request implements \phpbb\request\request_interface return $this->_variable($var_name, $default, $multibyte, $super_global, false); } + /** + * Get a variable without trimming strings and without escaping. + * This method MUST NOT be used with queries. + * Same functionality as variable(), except does not run trim() on strings + * and does not escape input. + * This method should only be used when the raw input is needed without + * any escaping, i.e. for database password during the installation. + * + * @param string|array $var_name The form variable's name from which data shall be retrieved. + * If the value is an array this may be an array of indizes which will give + * direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a") + * then specifying array("var", 1) as the name will return "a". + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * @param \phpbb\request\request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global should be used + * + * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the + * the same as that of $default. If the variable is not set $default is returned. + */ + public function raw_variable($var_name, $default, $super_global = \phpbb\request\request_interface::REQUEST) + { + $path = false; + + // deep direct access to multi dimensional arrays + if (is_array($var_name)) + { + $path = $var_name; + // make sure at least the variable name is specified + if (empty($path)) + { + return (is_array($default)) ? array() : $default; + } + // the variable name is the first element on the path + $var_name = array_shift($path); + } + + if (!isset($this->input[$super_global][$var_name])) + { + return (is_array($default)) ? array() : $default; + } + $var = $this->input[$super_global][$var_name]; + + if ($path) + { + // walk through the array structure and find the element we are looking for + foreach ($path as $key) + { + if (is_array($var) && isset($var[$key])) + { + $var = $var[$key]; + } + else + { + return (is_array($default)) ? array() : $default; + } + } + } + + return $var; + } + /** * Shortcut method to retrieve SERVER variables. * From 08bf8812d3bc7c22671e7e0dc88a0e99fcf403d7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 25 Nov 2016 22:51:29 +0100 Subject: [PATCH 3/5] [ticket/14875] Use raw_variable() method in _variable() to get raw data The raw_variable() method uses the same exact code the _variable method has been using until now. PHPBB3-14875 --- phpBB/phpbb/request/request.php | 39 +++++---------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/phpBB/phpbb/request/request.php b/phpBB/phpbb/request/request.php index 318d9f66f9..0d9ffa1780 100644 --- a/phpBB/phpbb/request/request.php +++ b/phpBB/phpbb/request/request.php @@ -431,41 +431,14 @@ class request implements \phpbb\request\request_interface */ protected function _variable($var_name, $default, $multibyte = false, $super_global = \phpbb\request\request_interface::REQUEST, $trim = true) { - $path = false; + $var = $this->raw_variable($var_name, $default, $super_global); - // deep direct access to multi dimensional arrays - if (is_array($var_name)) + // Return prematurely if raw variable is empty array or the same as + // the default. Using strict comparison to ensure that one can't + // prevent proper type checking on any input variable + if ($var === array() || $var === $default) { - $path = $var_name; - // make sure at least the variable name is specified - if (empty($path)) - { - return (is_array($default)) ? array() : $default; - } - // the variable name is the first element on the path - $var_name = array_shift($path); - } - - if (!isset($this->input[$super_global][$var_name])) - { - return (is_array($default)) ? array() : $default; - } - $var = $this->input[$super_global][$var_name]; - - if ($path) - { - // walk through the array structure and find the element we are looking for - foreach ($path as $key) - { - if (is_array($var) && isset($var[$key])) - { - $var = $var[$key]; - } - else - { - return (is_array($default)) ? array() : $default; - } - } + return $var; } $this->type_cast_helper->recursive_set_var($var, $default, $multibyte, $trim); From 9bdd002f584de78475362067b777749486504172 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 2 Dec 2016 11:36:07 +0100 Subject: [PATCH 4/5] [ticket/14875] Move raw_variable() method to request_interface PHPBB3-14875 --- .../helper/iohandler/ajax_iohandler.php | 6 ++--- .../helper/iohandler/iohandler_interface.php | 4 ++-- phpBB/phpbb/request/request.php | 19 +--------------- phpBB/phpbb/request/request_interface.php | 22 +++++++++++++++++++ 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php index 2db6750f3f..a40d457466 100644 --- a/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php +++ b/phpBB/phpbb/install/helper/iohandler/ajax_iohandler.php @@ -27,7 +27,7 @@ class ajax_iohandler extends iohandler_base protected $path_helper; /** - * @var \phpbb\request\request + * @var \phpbb\request\request_interface */ protected $request; @@ -90,12 +90,12 @@ class ajax_iohandler extends iohandler_base * Constructor * * @param path_helper $path_helper - * @param \phpbb\request\request $request HTTP request interface + * @param \phpbb\request\request_interface $request HTTP request interface * @param \phpbb\template\template $template Template engine * @param router $router Router * @param string $root_path Path to phpBB's root */ - public function __construct(path_helper $path_helper, \phpbb\request\request $request, \phpbb\template\template $template, router $router, $root_path) + public function __construct(path_helper $path_helper, \phpbb\request\request_interface $request, \phpbb\template\template $template, router $router, $root_path) { $this->path_helper = $path_helper; $this->request = $request; diff --git a/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php b/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php index f0e0e99bbb..440748901c 100644 --- a/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php +++ b/phpBB/phpbb/install/helper/iohandler/iohandler_interface.php @@ -52,7 +52,7 @@ interface iohandler_interface /** * Returns server variable * - * This function should work the same as request_interterface::server(). + * This function should work the same as request_interface::server(). * * @param string $name Name of the server variable * @param mixed $default Default value to return when the requested variable does not exist @@ -62,7 +62,7 @@ interface iohandler_interface public function get_server_variable($name, $default = ''); /** - * Wrapper function for request_interterface::header() + * Wrapper function for request_interface::header() * * @param string $name Name of the request header variable * @param mixed $default Default value to return when the requested variable does not exist diff --git a/phpBB/phpbb/request/request.php b/phpBB/phpbb/request/request.php index 0d9ffa1780..92d4213180 100644 --- a/phpBB/phpbb/request/request.php +++ b/phpBB/phpbb/request/request.php @@ -225,24 +225,7 @@ class request implements \phpbb\request\request_interface } /** - * Get a variable without trimming strings and without escaping. - * This method MUST NOT be used with queries. - * Same functionality as variable(), except does not run trim() on strings - * and does not escape input. - * This method should only be used when the raw input is needed without - * any escaping, i.e. for database password during the installation. - * - * @param string|array $var_name The form variable's name from which data shall be retrieved. - * If the value is an array this may be an array of indizes which will give - * direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a") - * then specifying array("var", 1) as the name will return "a". - * @param mixed $default A default value that is returned if the variable was not set. - * This function will always return a value of the same type as the default. - * @param \phpbb\request\request_interface::POST|GET|REQUEST|COOKIE $super_global - * Specifies which super global should be used - * - * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the - * the same as that of $default. If the variable is not set $default is returned. + * {@inheritdoc} */ public function raw_variable($var_name, $default, $super_global = \phpbb\request\request_interface::REQUEST) { diff --git a/phpBB/phpbb/request/request_interface.php b/phpBB/phpbb/request/request_interface.php index 47b3b3a4ed..3bfa8bb424 100644 --- a/phpBB/phpbb/request/request_interface.php +++ b/phpBB/phpbb/request/request_interface.php @@ -64,6 +64,28 @@ interface request_interface */ public function variable($var_name, $default, $multibyte = false, $super_global = \phpbb\request\request_interface::REQUEST); + /** + * Get a variable without trimming strings and without escaping. + * This method MUST NOT be used with queries. + * Same functionality as variable(), except does not run trim() on strings + * and does not escape input. + * This method should only be used when the raw input is needed without + * any escaping, i.e. for database password during the installation. + * + * @param string|array $var_name The form variable's name from which data shall be retrieved. + * If the value is an array this may be an array of indizes which will give + * direct access to a value at any depth. E.g. if the value of "var" is array(1 => "a") + * then specifying array("var", 1) as the name will return "a". + * @param mixed $default A default value that is returned if the variable was not set. + * This function will always return a value of the same type as the default. + * @param \phpbb\request\request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global should be used + * + * @return mixed The value of $_REQUEST[$var_name] run through {@link set_var set_var} to ensure that the type is the + * the same as that of $default. If the variable is not set $default is returned. + */ + public function raw_variable($var_name, $default, $super_global = \phpbb\request\request_interface::REQUEST); + /** * Shortcut method to retrieve SERVER variables. * From d817f3cc674433bfe3217b58fce887c48677b382 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 2 Dec 2016 12:53:17 +0100 Subject: [PATCH 5/5] [ticket/14875] Add raw_variable() to request mock PHPBB3-14875 --- tests/mock/request.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/mock/request.php b/tests/mock/request.php index e7217a94a9..6a32ba0cf1 100644 --- a/tests/mock/request.php +++ b/tests/mock/request.php @@ -34,6 +34,11 @@ class phpbb_mock_request implements \phpbb\request\request_interface $this->data[$super_global][$var_name] = $value; } + public function raw_variable($var_name, $default, $super_global = \phpbb\request\request_interface::REQUEST) + { + return $this->variable($var_name, $default, true, $super_global); + } + public function variable($var_name, $default, $multibyte = false, $super_global = \phpbb\request\request_interface::REQUEST) { return isset($this->data[$super_global][$var_name]) ? $this->data[$super_global][$var_name] : $default;