From 6f61169e41ed4115cc45db0b82d1acaecab0296d Mon Sep 17 00:00:00 2001 From: Dhruv Date: Wed, 15 Aug 2012 14:13:31 +0530 Subject: [PATCH 01/23] [ticket/11051] add public functions for public properties public retrieval functions for all public properties and change the properties to protected in all search backends. PHPBB3-11051 --- phpBB/includes/search/fulltext_mysql.php | 36 +++++++++++++++++++-- phpBB/includes/search/fulltext_native.php | 36 +++++++++++++++++++-- phpBB/includes/search/fulltext_postgres.php | 36 +++++++++++++++++++-- phpBB/includes/search/fulltext_sphinx.php | 26 +++++++++++++-- 4 files changed, 122 insertions(+), 12 deletions(-) diff --git a/phpBB/includes/search/fulltext_mysql.php b/phpBB/includes/search/fulltext_mysql.php index cf89ab1c24..7cd06dee19 100644 --- a/phpBB/includes/search/fulltext_mysql.php +++ b/phpBB/includes/search/fulltext_mysql.php @@ -27,9 +27,9 @@ class phpbb_search_fulltext_mysql extends phpbb_search_base protected $config; protected $db; protected $user; - public $word_length = array(); - public $search_query; - public $common_words = array(); + protected $word_length = array(); + protected $search_query; + protected $common_words = array(); /** * Constructor @@ -58,6 +58,36 @@ class phpbb_search_fulltext_mysql extends phpbb_search_base return 'MySQL Fulltext'; } + /** + * Returns the search_query + * + * @return string search query + */ + public function get_search_query() + { + return $this->search_query; + } + + /** + * Returns the common_words array + * + * @return array common words that are ignored by search backend + */ + public function get_common_words() + { + return $this->common_words; + } + + /** + * Returns the word_length array + * + * @return array min and max word length for searching + */ + public function get_word_length() + { + return $this->word_length; + } + /** * Checks for correct MySQL version and stores min/max word length in the config * diff --git a/phpBB/includes/search/fulltext_native.php b/phpBB/includes/search/fulltext_native.php index 96b3f02ec6..1100484ebd 100644 --- a/phpBB/includes/search/fulltext_native.php +++ b/phpBB/includes/search/fulltext_native.php @@ -23,9 +23,9 @@ if (!defined('IN_PHPBB')) class phpbb_search_fulltext_native extends phpbb_search_base { protected $stats = array(); - public $word_length = array(); - public $search_query; - public $common_words = array(); + protected $word_length = array(); + protected $search_query; + protected $common_words = array(); protected $must_contain_ids = array(); protected $must_not_contain_ids = array(); @@ -73,6 +73,36 @@ class phpbb_search_fulltext_native extends phpbb_search_base return 'phpBB Native Fulltext'; } + /** + * Returns the search_query + * + * @return string search query + */ + public function get_search_query() + { + return $this->search_query; + } + + /** + * Returns the common_words array + * + * @return array common words that are ignored by search backend + */ + public function get_common_words() + { + return $this->common_words; + } + + /** + * Returns the word_length array + * + * @return array min and max word length for searching + */ + public function get_word_length() + { + return $this->word_length; + } + /** * This function fills $this->search_query with the cleaned user search query. * diff --git a/phpBB/includes/search/fulltext_postgres.php b/phpBB/includes/search/fulltext_postgres.php index 50ed785093..38989a9d9a 100644 --- a/phpBB/includes/search/fulltext_postgres.php +++ b/phpBB/includes/search/fulltext_postgres.php @@ -31,9 +31,9 @@ class phpbb_search_fulltext_postgres extends phpbb_search_base protected $config; protected $db; protected $user; - public $search_query; - public $common_words = array(); - public $word_length = array(); + protected $search_query; + protected $common_words = array(); + protected $word_length = array(); /** * Constructor @@ -72,6 +72,36 @@ class phpbb_search_fulltext_postgres extends phpbb_search_base return 'PostgreSQL Fulltext'; } + /** + * Returns the search_query + * + * @return string search query + */ + public function get_search_query() + { + return $this->search_query; + } + + /** + * Returns the common_words array + * + * @return array common words that are ignored by search backend + */ + public function get_common_words() + { + return $this->common_words; + } + + /** + * Returns the word_length array + * + * @return array min and max word length for searching + */ + public function get_word_length() + { + return $this->word_length; + } + /** * Returns if phrase search is supported or not * diff --git a/phpBB/includes/search/fulltext_sphinx.php b/phpBB/includes/search/fulltext_sphinx.php index 1a3bbe23db..256c3a6d1c 100644 --- a/phpBB/includes/search/fulltext_sphinx.php +++ b/phpBB/includes/search/fulltext_sphinx.php @@ -42,8 +42,8 @@ class phpbb_search_fulltext_sphinx protected $dbtype; protected $user; protected $config_file_data = ''; - public $search_query; - public $common_words = array(); + protected $search_query; + protected $common_words = array(); /** * Constructor @@ -87,7 +87,7 @@ class phpbb_search_fulltext_sphinx $error = false; } - + /** * Returns the name of this search backend to be displayed to administrators * @@ -98,6 +98,26 @@ class phpbb_search_fulltext_sphinx return 'Sphinx Fulltext'; } + /** + * Returns the search_query + * + * @return string search query + */ + public function get_search_query() + { + return $this->search_query; + } + + /** + * Returns the common_words array + * + * @return array common words that are ignored by search backend + */ + public function get_common_words() + { + return $this->common_words; + } + /** * Checks permissions and paths, if everything is correct it generates the config file * From 7402328b8177a2080cc269be12aeb84f54d65a22 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 16 Aug 2012 22:35:50 +0530 Subject: [PATCH 02/23] [ticket/11051] function instead of accessing property in search Use get_search_query() instead of accessing the search_query property directly as it is now protected. PHPBB3-11051 --- phpBB/search.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/search.php b/phpBB/search.php index 190da5247f..c6c92e68b0 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -291,7 +291,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) if ($keywords) { $correct_query = $search->split_keywords($keywords, $search_terms); - if (!$correct_query || (empty($search->search_query) && !sizeof($author_id_ary) && !$search_id)) + if (!$correct_query || (!$search->get_search_query() && !sizeof($author_id_ary) && !$search_id)) { $ignored = (sizeof($search->common_words)) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->common_words)) . '
' : ''; trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $search->word_length['min']), $user->lang('CHARACTERS', (int) $search->word_length['max']))); @@ -526,7 +526,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) sort($m_approve_fid_ary); sort($author_id_ary); - if (!empty($search->search_query)) + if ($search->get_search_query()) { $total_match_count = $search->keyword_search($show_results, $search_fields, $search_terms, $sort_by_sql, $sort_key, $sort_dir, $sort_days, $ex_fid_ary, $m_approve_fid_ary, $topic_id, $author_id_ary, $sql_author_match, $id_ary, $start, $per_page); } @@ -609,7 +609,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) 'SEARCH_TITLE' => $l_search_title, 'SEARCH_MATCHES' => $l_search_matches, 'SEARCH_WORDS' => $keywords, - 'SEARCHED_QUERY' => $search->search_query, + 'SEARCHED_QUERY' => $search->get_search_query(), 'IGNORED_WORDS' => (sizeof($search->common_words)) ? implode(' ', $search->common_words) : '', 'PAGE_NUMBER' => phpbb_on_page($template, $user, $u_search, $total_match_count, $per_page, $start), From 9b9dc2fab71ec95c4dc48787dd9950872773cfa7 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 16 Aug 2012 23:16:26 +0530 Subject: [PATCH 03/23] [ticket/11051] use get_common_words in search backend Use get_common_words() instead of accessing the common_words property directly as it is now protected. PHPBB3-11051 --- phpBB/search.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/search.php b/phpBB/search.php index c6c92e68b0..fd67b483e8 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -293,7 +293,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) $correct_query = $search->split_keywords($keywords, $search_terms); if (!$correct_query || (!$search->get_search_query() && !sizeof($author_id_ary) && !$search_id)) { - $ignored = (sizeof($search->common_words)) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->common_words)) . '
' : ''; + $ignored = (sizeof($search->get_common_words())) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->get_common_words())) . '
' : ''; trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $search->word_length['min']), $user->lang('CHARACTERS', (int) $search->word_length['max']))); } } @@ -610,7 +610,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) 'SEARCH_MATCHES' => $l_search_matches, 'SEARCH_WORDS' => $keywords, 'SEARCHED_QUERY' => $search->get_search_query(), - 'IGNORED_WORDS' => (sizeof($search->common_words)) ? implode(' ', $search->common_words) : '', + 'IGNORED_WORDS' => (sizeof($search->get_common_words())) ? implode(' ', $search->get_common_words()) : '', 'PAGE_NUMBER' => phpbb_on_page($template, $user, $u_search, $total_match_count, $per_page, $start), 'PHRASE_SEARCH_DISABLED' => $phrase_search_disabled, From a2e1989b012e3f77a18c2758cd913245ce3c180b Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 17 Aug 2012 00:11:32 +0530 Subject: [PATCH 04/23] [ticket/11051] use get_word_length in search backend Use get_word_length() instead od accessing the word_length property directly as it is now protected. PHPBB3-11051 --- phpBB/search.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/search.php b/phpBB/search.php index fd67b483e8..54f0f225a0 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -294,7 +294,9 @@ if ($keywords || $author || $author_id || $search_id || $submit) if (!$correct_query || (!$search->get_search_query() && !sizeof($author_id_ary) && !$search_id)) { $ignored = (sizeof($search->get_common_words())) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->get_common_words())) . '
' : ''; - trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $search->word_length['min']), $user->lang('CHARACTERS', (int) $search->word_length['max']))); + // Function cannot be directly used as associative array for php < 5.4 + $word_length = $search->get_word_length(); + trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $word_length['min']), $user->lang('CHARACTERS', (int) $word_length['max']))); } } From c83ca457a002be4899390cc8a7b0c2636f69e25d Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 26 Aug 2012 17:20:45 +0200 Subject: [PATCH 05/23] [ticket/11069] Add closing span back to credit line in subsilver2 simple footer Regression from 0ceb77fb995e7433242a87c1fe0c22840a2b23e1. PHPBB3-11069 --- phpBB/styles/subsilver2/template/simple_footer.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/styles/subsilver2/template/simple_footer.html b/phpBB/styles/subsilver2/template/simple_footer.html index db95c7952a..6cb4f401da 100644 --- a/phpBB/styles/subsilver2/template/simple_footer.html +++ b/phpBB/styles/subsilver2/template/simple_footer.html @@ -2,7 +2,7 @@
- {CREDIT_LINE} + {CREDIT_LINE}
From b3cd5a649be62f175de651a16ae02c5f709ca2f4 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Mon, 3 Sep 2012 13:32:33 -0500 Subject: [PATCH 06/23] [ticket/8713] Do not trim login inputs Create a function to request variables which are not trimmed. All requests for passwords (except forum passwords) now use the untrimmed request function. PHPBB3-8713 --- phpBB/includes/acp/acp_language.php | 6 +- phpBB/includes/acp/acp_users.php | 6 +- phpBB/includes/functions.php | 4 +- phpBB/includes/request/request.php | 63 +++++++++++++++++++++ phpBB/includes/request/type_cast_helper.php | 22 +++++-- phpBB/includes/ucp/ucp_profile.php | 6 +- phpBB/includes/ucp/ucp_register.php | 4 +- phpBB/install/install_update.php | 4 +- tests/request/type_cast_helper_test.php | 10 ++++ 9 files changed, 104 insertions(+), 21 deletions(-) diff --git a/phpBB/includes/acp/acp_language.php b/phpBB/includes/acp/acp_language.php index 2b19f93c75..87cf605d8e 100644 --- a/phpBB/includes/acp/acp_language.php +++ b/phpBB/includes/acp/acp_language.php @@ -100,11 +100,11 @@ class acp_language switch ($method) { case 'ftp': - $transfer = new ftp(request_var('host', ''), request_var('username', ''), request_var('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); break; case 'ftp_fsock': - $transfer = new ftp_fsock(request_var('host', ''), request_var('username', ''), request_var('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp_fsock(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); break; default: @@ -404,7 +404,7 @@ class acp_language trigger_error($user->lang['INVALID_UPLOAD_METHOD'], E_USER_ERROR); } - $transfer = new $method(request_var('host', ''), request_var('username', ''), request_var('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); if (($result = $transfer->open_session()) !== true) { diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index b54257b04a..b9958ed0f1 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -32,7 +32,7 @@ class acp_users { global $config, $db, $user, $auth, $template, $cache; global $phpbb_root_path, $phpbb_admin_path, $phpEx, $table_prefix, $file_uploads; - global $phpbb_dispatcher; + global $phpbb_dispatcher, $request; $user->add_lang(array('posting', 'ucp', 'acp/users')); $this->tpl_name = 'acp_users'; @@ -770,8 +770,8 @@ class acp_users 'username' => utf8_normalize_nfc(request_var('user', $user_row['username'], true)), 'user_founder' => request_var('user_founder', ($user_row['user_type'] == USER_FOUNDER) ? 1 : 0), 'email' => strtolower(request_var('user_email', $user_row['user_email'])), - 'new_password' => request_var('new_password', '', true), - 'password_confirm' => request_var('password_confirm', '', true), + 'new_password' => $request->untrimed_variable('new_password', '', true), + 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), ); // Validation data - we do not check the password complexity setting here diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 834f57a38b..1cdda60855 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3044,11 +3044,11 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa trigger_error('NO_AUTH_ADMIN'); } - $password = request_var('password_' . $credential, '', true); + $password = $request->untrimed_variable('password_' . $credential, '', true); } else { - $password = request_var('password', '', true); + $password = $request->untrimed_variable('password', '', true); } $username = request_var('username', '', true); diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index 4e425dbd27..747ca09624 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -242,6 +242,69 @@ class phpbb_request implements phpbb_request_interface return $var; } + /** + * Get a variable, but without trimming strings + * Same functionality as variable(), except does not run trim() on strings + * All variables in GET or POST requests should be retrieved through this function to maximise security. + * + * @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 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 + * @param phpbb_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 untrimed_variable($var_name, $default, $multibyte, $super_global = phpbb_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; + } + } + } + + $this->type_cast_helper->recursive_set_var($var, $default, $multibyte, false); + + return $var; + } + /** * Shortcut method to retrieve SERVER variables. * diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index 561e8fc251..d3b94aac5a 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -93,15 +93,23 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i * @param mixed $type The variable type. Will be used with {@link settype()} * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks. + * @param bool $trim Indicates whether string values will be be parsed with trim() + * Default is true */ - public function set_var(&$result, $var, $type, $multibyte = false) + public function set_var(&$result, $var, $type, $multibyte = false, $trim = true) { settype($var, $type); $result = $var; if ($type == 'string') { - $result = trim(str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result)); + $result = str_replace(array("\r\n", "\r", "\0"), array("\n", "\n", ''), $result); + + if ($trim) + { + $result = trim($result); + } + $result = htmlspecialchars($result, ENT_COMPAT, 'UTF-8'); if ($multibyte) @@ -141,8 +149,10 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i * @param bool $multibyte Indicates whether string keys and values may contain UTF-8 characters. * Default is false, causing all bytes outside the ASCII range (0-127) to * be replaced with question marks. + * @param bool $trim Indicates whether string values will be be parsed with trim() + * Default is true */ - public function recursive_set_var(&$var, $default, $multibyte) + public function recursive_set_var(&$var, $default, $multibyte, $trim = true) { if (is_array($var) !== is_array($default)) { @@ -153,7 +163,7 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i if (!is_array($default)) { $type = gettype($default); - $this->set_var($var, $var, $type, $multibyte); + $this->set_var($var, $var, $type, $multibyte, $trim); } else { @@ -174,9 +184,9 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i foreach ($_var as $k => $v) { - $this->set_var($k, $k, $key_type, $multibyte, $multibyte); + $this->set_var($k, $k, $key_type, $multibyte, $trim); - $this->recursive_set_var($v, $default_value, $multibyte); + $this->recursive_set_var($v, $default_value, $multibyte, $trim); $var[$k] = $v; } } diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index 2ac82fb52f..68d5dd5d65 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -46,9 +46,9 @@ class ucp_profile $data = array( 'username' => utf8_normalize_nfc(request_var('username', $user->data['username'], true)), 'email' => strtolower(request_var('email', $user->data['user_email'])), - 'new_password' => request_var('new_password', '', true), - 'cur_password' => request_var('cur_password', '', true), - 'password_confirm' => request_var('password_confirm', '', true), + 'new_password' => $request->untrimed_variable('new_password', '', true), + 'cur_password' => $request->untrimed_variable('cur_password', '', true), + 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), ); add_form_key('ucp_reg_details'); diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 6ce53a79ab..6fab189a99 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -170,8 +170,8 @@ class ucp_register $data = array( 'username' => utf8_normalize_nfc(request_var('username', '', true)), - 'new_password' => request_var('new_password', '', true), - 'password_confirm' => request_var('password_confirm', '', true), + 'new_password' => $request->untrimed_variable('new_password', '', true), + 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), 'email' => strtolower(request_var('email', '')), 'lang' => basename(request_var('lang', $user->lang_name)), 'tz' => request_var('tz', $timezone), diff --git a/phpBB/install/install_update.php b/phpBB/install/install_update.php index 88b00f1cf1..4b5a23e497 100644 --- a/phpBB/install/install_update.php +++ b/phpBB/install/install_update.php @@ -862,7 +862,7 @@ class install_update extends module $test_connection = false; if ($test_ftp_connection || $submit) { - $transfer = new $method(request_var('host', ''), request_var('username', ''), request_var('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); $test_connection = $transfer->open_session(); // Make sure that the directory is correct by checking for the existence of common.php @@ -948,7 +948,7 @@ class install_update extends module } else { - $transfer = new $method(request_var('host', ''), request_var('username', ''), request_var('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); $transfer->open_session(); } diff --git a/tests/request/type_cast_helper_test.php b/tests/request/type_cast_helper_test.php index d553d5b8cd..f7e5cd873e 100644 --- a/tests/request/type_cast_helper_test.php +++ b/tests/request/type_cast_helper_test.php @@ -48,4 +48,14 @@ class phpbb_type_cast_helper_test extends phpbb_test_case $this->assertEquals($expected, $data); } + + public function test_untrimmed_strings() + { + $data = array(' eviL<3 '); + $expected = array(' eviL<3 '); + + $this->type_cast_helper->recursive_set_var($data, '', true, false); + + $this->assertEquals($expected, $data); + } } From 552233d8fd27ca09fbed555582a9880771205929 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 01:03:38 +0200 Subject: [PATCH 07/23] [ticket/11100] Mark can_use_ssl() and can_use_tls() as static. PHPBB3-11100 --- phpBB/includes/functions_jabber.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/functions_jabber.php b/phpBB/includes/functions_jabber.php index d76309d5bb..3d8e403f4b 100644 --- a/phpBB/includes/functions_jabber.php +++ b/phpBB/includes/functions_jabber.php @@ -68,7 +68,7 @@ class jabber } $this->password = $password; - $this->use_ssl = ($use_ssl && $this->can_use_ssl()) ? true : false; + $this->use_ssl = ($use_ssl && self::can_use_ssl()) ? true : false; // Change port if we use SSL if ($this->port == 5222 && $this->use_ssl) @@ -83,7 +83,7 @@ class jabber /** * Able to use the SSL functionality? */ - function can_use_ssl() + static public function can_use_ssl() { // Will not work with PHP >= 5.2.1 or < 5.2.3RC2 until timeout problem with ssl hasn't been fixed (http://bugs.php.net/41236) return ((version_compare(PHP_VERSION, '5.2.1', '<') || version_compare(PHP_VERSION, '5.2.3RC2', '>=')) && @extension_loaded('openssl')) ? true : false; @@ -92,7 +92,7 @@ class jabber /** * Able to use TLS? */ - function can_use_tls() + static public function can_use_tls() { if (!@extension_loaded('openssl') || !function_exists('stream_socket_enable_crypto') || !function_exists('stream_get_meta_data') || !function_exists('socket_set_blocking') || !function_exists('stream_get_wrappers')) { @@ -442,7 +442,7 @@ class jabber } // Let's use TLS if SSL is not enabled and we can actually use it - if (!$this->session['ssl'] && $this->can_use_tls() && $this->can_use_ssl() && isset($xml['stream:features'][0]['#']['starttls'])) + if (!$this->session['ssl'] && self::can_use_tls() && self::can_use_ssl() && isset($xml['stream:features'][0]['#']['starttls'])) { $this->add_to_log('Switching to TLS.'); $this->send("\n"); From 06c3868c27c394747bbaa5a8dac6ed83b5d61951 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 13:40:28 +0200 Subject: [PATCH 08/23] [ticket/8713] Adjust test method name to other recursive_set_var() tests. PHPBB3-8713 --- tests/request/type_cast_helper_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/request/type_cast_helper_test.php b/tests/request/type_cast_helper_test.php index f7e5cd873e..94b6e9146f 100644 --- a/tests/request/type_cast_helper_test.php +++ b/tests/request/type_cast_helper_test.php @@ -49,7 +49,7 @@ class phpbb_type_cast_helper_test extends phpbb_test_case $this->assertEquals($expected, $data); } - public function test_untrimmed_strings() + public function test_nested_untrimmed_recursive_set_var() { $data = array(' eviL<3 '); $expected = array(' eviL<3 '); From 2c41b9062a6a8335aa1bfa7c80077f4ae33d33e4 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 13:41:51 +0200 Subject: [PATCH 09/23] [ticket/8713] Use correct parameter for nested data. PHPBB3-8713 --- tests/request/type_cast_helper_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/request/type_cast_helper_test.php b/tests/request/type_cast_helper_test.php index 94b6e9146f..176638dc44 100644 --- a/tests/request/type_cast_helper_test.php +++ b/tests/request/type_cast_helper_test.php @@ -54,7 +54,7 @@ class phpbb_type_cast_helper_test extends phpbb_test_case $data = array(' eviL<3 '); $expected = array(' eviL<3 '); - $this->type_cast_helper->recursive_set_var($data, '', true, false); + $this->type_cast_helper->recursive_set_var($data, array(0 => ''), true, false); $this->assertEquals($expected, $data); } From 4550fff55a10be737b76275ae5323675ab1c3939 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 13:43:14 +0200 Subject: [PATCH 10/23] [ticket/8713] Use \t in double quotes instead of tabs. PHPBB3-8713 --- tests/request/type_cast_helper_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/request/type_cast_helper_test.php b/tests/request/type_cast_helper_test.php index 176638dc44..8203703639 100644 --- a/tests/request/type_cast_helper_test.php +++ b/tests/request/type_cast_helper_test.php @@ -51,8 +51,8 @@ class phpbb_type_cast_helper_test extends phpbb_test_case public function test_nested_untrimmed_recursive_set_var() { - $data = array(' eviL<3 '); - $expected = array(' eviL<3 '); + $data = array(" eviL<3\t\t"); + $expected = array(" eviL<3\t\t"); $this->type_cast_helper->recursive_set_var($data, array(0 => ''), true, false); From 160c49351b5ce7d2d811a388a4630ec37258bb8f Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 13:44:50 +0200 Subject: [PATCH 11/23] [ticket/8713] Add simple (non-nested) test case for untrimmed set_var(). PHPBB3-8713 --- tests/request/type_cast_helper_test.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/request/type_cast_helper_test.php b/tests/request/type_cast_helper_test.php index 8203703639..f41db005af 100644 --- a/tests/request/type_cast_helper_test.php +++ b/tests/request/type_cast_helper_test.php @@ -49,6 +49,16 @@ class phpbb_type_cast_helper_test extends phpbb_test_case $this->assertEquals($expected, $data); } + public function test_simple_untrimmed_recursive_set_var() + { + $data = " eviL<3\t\t"; + $expected = " eviL<3\t\t"; + + $this->type_cast_helper->recursive_set_var($data, '', true, false); + + $this->assertEquals($expected, $data); + } + public function test_nested_untrimmed_recursive_set_var() { $data = array(" eviL<3\t\t"); From 798033075ba0bbef8f43c542ca05aae776747917 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 14:07:06 +0200 Subject: [PATCH 12/23] [ticket/8713] Always trim array keys. PHPBB3-8713 --- phpBB/includes/request/type_cast_helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index d3b94aac5a..3039647bfa 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -184,7 +184,7 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i foreach ($_var as $k => $v) { - $this->set_var($k, $k, $key_type, $multibyte, $trim); + $this->set_var($k, $k, $key_type, $multibyte); $this->recursive_set_var($v, $default_value, $multibyte, $trim); $var[$k] = $v; From c3e0d1b6d12f07df88e31ffd896b275e65b788eb Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 14:07:42 +0200 Subject: [PATCH 13/23] [ticket/8713] Fix type_cast_helper.php doc blocks: Add punctuation etc. PHPBB3-8713 --- phpBB/includes/request/type_cast_helper.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/request/type_cast_helper.php b/phpBB/includes/request/type_cast_helper.php index 3039647bfa..1a5274ed14 100644 --- a/phpBB/includes/request/type_cast_helper.php +++ b/phpBB/includes/request/type_cast_helper.php @@ -93,8 +93,8 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i * @param mixed $type The variable type. Will be used with {@link settype()} * @param bool $multibyte Indicates whether string values may contain UTF-8 characters. * Default is false, causing all bytes outside the ASCII range (0-127) to be replaced with question marks. - * @param bool $trim Indicates whether string values will be be parsed with trim() - * Default is true + * @param bool $trim Indicates whether trim() should be applied to string values. + * Default is true. */ public function set_var(&$result, $var, $type, $multibyte = false, $trim = true) { @@ -149,8 +149,8 @@ class phpbb_request_type_cast_helper implements phpbb_request_type_cast_helper_i * @param bool $multibyte Indicates whether string keys and values may contain UTF-8 characters. * Default is false, causing all bytes outside the ASCII range (0-127) to * be replaced with question marks. - * @param bool $trim Indicates whether string values will be be parsed with trim() - * Default is true + * @param bool $trim Indicates whether trim() should be applied to string values. + * Default is true. */ public function recursive_set_var(&$var, $default, $multibyte, $trim = true) { From b62c37c5799d4b9e018358c38a731d6664acadf1 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 14:09:12 +0200 Subject: [PATCH 14/23] [ticket/8713] DRY: variable() and untrimed_variable() into a protected method. PHPBB3-8713 --- phpBB/includes/request/request.php | 144 +++++++++++++---------------- 1 file changed, 64 insertions(+), 80 deletions(-) diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index 747ca09624..c6b6610af5 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -200,46 +200,7 @@ class phpbb_request implements phpbb_request_interface */ public function variable($var_name, $default, $multibyte = false, $super_global = phpbb_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; - } - } - } - - $this->type_cast_helper->recursive_set_var($var, $default, $multibyte); - - return $var; + return $this->_variable($var_name, $default, $multibyte, $super_global, true); } /** @@ -263,46 +224,7 @@ class phpbb_request implements phpbb_request_interface */ public function untrimed_variable($var_name, $default, $multibyte, $super_global = phpbb_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; - } - } - } - - $this->type_cast_helper->recursive_set_var($var, $default, $multibyte, false); - - return $var; + return $this->_variable($var_name, $default, $multibyte, $super_global, false); } /** @@ -414,4 +336,66 @@ class phpbb_request implements phpbb_request_interface return array_keys($this->input[$super_global]); } + + /** + * Helper function used by variable() and untrimed_variable(). + * + * @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 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 + * @param phpbb_request_interface::POST|GET|REQUEST|COOKIE $super_global + * Specifies which super global should be used + * @param bool $trim Indicates whether trim() should be applied to string values. + * + * @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. + */ + protected function _variable($var_name, $default, $multibyte = false, $super_global = phpbb_request_interface::REQUEST, $trim = true) + { + $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; + } + } + } + + $this->type_cast_helper->recursive_set_var($var, $default, $multibyte, $trim); + + return $var; + } } From f2607fc9e80c6f9ad7543b7be5ea6f294aa6c40a Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 14:15:56 +0200 Subject: [PATCH 15/23] [ticket/8713] Rename untrimed_variable() to untrimmed_variable(). PHPBB3-8713 --- phpBB/includes/acp/acp_language.php | 6 +++--- phpBB/includes/acp/acp_users.php | 4 ++-- phpBB/includes/functions.php | 4 ++-- phpBB/includes/request/request.php | 4 ++-- phpBB/includes/ucp/ucp_profile.php | 6 +++--- phpBB/includes/ucp/ucp_register.php | 4 ++-- phpBB/install/install_update.php | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/phpBB/includes/acp/acp_language.php b/phpBB/includes/acp/acp_language.php index 87cf605d8e..b5f5ba2312 100644 --- a/phpBB/includes/acp/acp_language.php +++ b/phpBB/includes/acp/acp_language.php @@ -100,11 +100,11 @@ class acp_language switch ($method) { case 'ftp': - $transfer = new ftp(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); break; case 'ftp_fsock': - $transfer = new ftp_fsock(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp_fsock(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); break; default: @@ -404,7 +404,7 @@ class acp_language trigger_error($user->lang['INVALID_UPLOAD_METHOD'], E_USER_ERROR); } - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); if (($result = $transfer->open_session()) !== true) { diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index b9958ed0f1..2905b84d57 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -770,8 +770,8 @@ class acp_users 'username' => utf8_normalize_nfc(request_var('user', $user_row['username'], true)), 'user_founder' => request_var('user_founder', ($user_row['user_type'] == USER_FOUNDER) ? 1 : 0), 'email' => strtolower(request_var('user_email', $user_row['user_email'])), - 'new_password' => $request->untrimed_variable('new_password', '', true), - 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), + 'new_password' => $request->untrimmed_variable('new_password', '', true), + 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), ); // Validation data - we do not check the password complexity setting here diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 1cdda60855..a2f8a57938 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3044,11 +3044,11 @@ function login_box($redirect = '', $l_explain = '', $l_success = '', $admin = fa trigger_error('NO_AUTH_ADMIN'); } - $password = $request->untrimed_variable('password_' . $credential, '', true); + $password = $request->untrimmed_variable('password_' . $credential, '', true); } else { - $password = $request->untrimed_variable('password', '', true); + $password = $request->untrimmed_variable('password', '', true); } $username = request_var('username', '', true); diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index c6b6610af5..aa62c3b610 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -222,7 +222,7 @@ class phpbb_request implements phpbb_request_interface * @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 untrimed_variable($var_name, $default, $multibyte, $super_global = phpbb_request_interface::REQUEST) + public function untrimmed_variable($var_name, $default, $multibyte, $super_global = phpbb_request_interface::REQUEST) { return $this->_variable($var_name, $default, $multibyte, $super_global, false); } @@ -338,7 +338,7 @@ class phpbb_request implements phpbb_request_interface } /** - * Helper function used by variable() and untrimed_variable(). + * Helper function used by variable() and untrimmed_variable(). * * @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 diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index 68d5dd5d65..db1e3e4722 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -46,9 +46,9 @@ class ucp_profile $data = array( 'username' => utf8_normalize_nfc(request_var('username', $user->data['username'], true)), 'email' => strtolower(request_var('email', $user->data['user_email'])), - 'new_password' => $request->untrimed_variable('new_password', '', true), - 'cur_password' => $request->untrimed_variable('cur_password', '', true), - 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), + 'new_password' => $request->untrimmed_variable('new_password', '', true), + 'cur_password' => $request->untrimmed_variable('cur_password', '', true), + 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), ); add_form_key('ucp_reg_details'); diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 6fab189a99..5ae92a5cea 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -170,8 +170,8 @@ class ucp_register $data = array( 'username' => utf8_normalize_nfc(request_var('username', '', true)), - 'new_password' => $request->untrimed_variable('new_password', '', true), - 'password_confirm' => $request->untrimed_variable('password_confirm', '', true), + 'new_password' => $request->untrimmed_variable('new_password', '', true), + 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), 'email' => strtolower(request_var('email', '')), 'lang' => basename(request_var('lang', $user->lang_name)), 'tz' => request_var('tz', $timezone), diff --git a/phpBB/install/install_update.php b/phpBB/install/install_update.php index 4b5a23e497..1ecedecce6 100644 --- a/phpBB/install/install_update.php +++ b/phpBB/install/install_update.php @@ -862,7 +862,7 @@ class install_update extends module $test_connection = false; if ($test_ftp_connection || $submit) { - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); $test_connection = $transfer->open_session(); // Make sure that the directory is correct by checking for the existence of common.php @@ -948,7 +948,7 @@ class install_update extends module } else { - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); $transfer->open_session(); } From cc0c378caf9bfc480391a9d11d5a4d78c0df097c Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 14:40:35 +0200 Subject: [PATCH 16/23] [ticket/8713] Call htmlspecialchars_decode() on transfer (e.g. ftp) passwords. PHPBB3-8713 --- phpBB/includes/acp/acp_language.php | 27 ++++++++++++++++++++++++--- phpBB/install/install_update.php | 18 ++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/acp/acp_language.php b/phpBB/includes/acp/acp_language.php index b5f5ba2312..2be1ccfc41 100644 --- a/phpBB/includes/acp/acp_language.php +++ b/phpBB/includes/acp/acp_language.php @@ -100,11 +100,25 @@ class acp_language switch ($method) { case 'ftp': - $transfer = new ftp(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp( + request_var('host', ''), + request_var('username', ''), + htmlspecialchars_decode($request->untrimmed_variable('password', '')), + request_var('root_path', ''), + request_var('port', ''), + request_var('timeout', '') + ); break; case 'ftp_fsock': - $transfer = new ftp_fsock(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new ftp_fsock( + request_var('host', ''), + request_var('username', ''), + htmlspecialchars_decode($request->untrimmed_variable('password', '')), + request_var('root_path', ''), + request_var('port', ''), + request_var('timeout', '') + ); break; default: @@ -404,7 +418,14 @@ class acp_language trigger_error($user->lang['INVALID_UPLOAD_METHOD'], E_USER_ERROR); } - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method( + request_var('host', ''), + request_var('username', ''), + htmlspecialchars_decode($request->untrimmed_variable('password', '')), + request_var('root_path', ''), + request_var('port', ''), + request_var('timeout', '') + ); if (($result = $transfer->open_session()) !== true) { diff --git a/phpBB/install/install_update.php b/phpBB/install/install_update.php index 1ecedecce6..8c044550f3 100644 --- a/phpBB/install/install_update.php +++ b/phpBB/install/install_update.php @@ -862,7 +862,14 @@ class install_update extends module $test_connection = false; if ($test_ftp_connection || $submit) { - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method( + request_var('host', ''), + request_var('username', ''), + htmlspecialchars_decode($request->untrimmed_variable('password', '')), + request_var('root_path', ''), + request_var('port', ''), + request_var('timeout', '') + ); $test_connection = $transfer->open_session(); // Make sure that the directory is correct by checking for the existence of common.php @@ -948,7 +955,14 @@ class install_update extends module } else { - $transfer = new $method(request_var('host', ''), request_var('username', ''), $request->untrimmed_variable('password', ''), request_var('root_path', ''), request_var('port', ''), request_var('timeout', '')); + $transfer = new $method( + request_var('host', ''), + request_var('username', ''), + htmlspecialchars_decode($request->untrimmed_variable('password', '')), + request_var('root_path', ''), + request_var('port', ''), + request_var('timeout', '') + ); $transfer->open_session(); } From 1e05fd4c627d23b7756796c5acac27d2562a8607 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 15:01:29 +0200 Subject: [PATCH 17/23] [ticket/8713] Trim password in auth_db to keep compatibility. PHPBB3-8713 --- phpBB/includes/auth/auth_db.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/auth/auth_db.php b/phpBB/includes/auth/auth_db.php index 76790e4dad..ac944532a5 100644 --- a/phpBB/includes/auth/auth_db.php +++ b/phpBB/includes/auth/auth_db.php @@ -41,6 +41,10 @@ function login_db($username, $password, $ip = '', $browser = '', $forwarded_for global $db, $config; global $request; + // Auth plugins get the password untrimmed. + // For compatibility we trim() here. + $password = trim($password); + // do not allow empty password if (!$password) { From 73a75fc3d387f8d923186c5c04b1ca7bc6cda4ef Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 8 Sep 2012 15:02:06 +0200 Subject: [PATCH 18/23] [ticket/8713] Revert changes to ucp_profile, ucp_register and acp_users. Currently only auth_db is supported there and the password needs to be trimmed for compatibility because user_password stores phpbb_hash(htmlspecialchars(trim($password))) Setting passwords for other auth modules is currently not supported. Once setting/changing passwords is supported by auth plugins, the untrimmed_variable() should be used here and the result should be passed to the auth plugin. PHPBB3-8713 --- phpBB/includes/acp/acp_users.php | 4 ++-- phpBB/includes/ucp/ucp_profile.php | 6 +++--- phpBB/includes/ucp/ucp_register.php | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 2905b84d57..985a12d9ce 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -770,8 +770,8 @@ class acp_users 'username' => utf8_normalize_nfc(request_var('user', $user_row['username'], true)), 'user_founder' => request_var('user_founder', ($user_row['user_type'] == USER_FOUNDER) ? 1 : 0), 'email' => strtolower(request_var('user_email', $user_row['user_email'])), - 'new_password' => $request->untrimmed_variable('new_password', '', true), - 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), + 'new_password' => $request->variable('new_password', '', true), + 'password_confirm' => $request->variable('password_confirm', '', true), ); // Validation data - we do not check the password complexity setting here diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index db1e3e4722..89bf20a30f 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -46,9 +46,9 @@ class ucp_profile $data = array( 'username' => utf8_normalize_nfc(request_var('username', $user->data['username'], true)), 'email' => strtolower(request_var('email', $user->data['user_email'])), - 'new_password' => $request->untrimmed_variable('new_password', '', true), - 'cur_password' => $request->untrimmed_variable('cur_password', '', true), - 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), + 'new_password' => $request->variable('new_password', '', true), + 'cur_password' => $request->variable('cur_password', '', true), + 'password_confirm' => $request->variable('password_confirm', '', true), ); add_form_key('ucp_reg_details'); diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 5ae92a5cea..c57aec00a0 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -170,8 +170,8 @@ class ucp_register $data = array( 'username' => utf8_normalize_nfc(request_var('username', '', true)), - 'new_password' => $request->untrimmed_variable('new_password', '', true), - 'password_confirm' => $request->untrimmed_variable('password_confirm', '', true), + 'new_password' => $request->variable('new_password', '', true), + 'password_confirm' => $request->variable('password_confirm', '', true), 'email' => strtolower(request_var('email', '')), 'lang' => basename(request_var('lang', $user->lang_name)), 'tz' => request_var('tz', $timezone), From 238fab3bb908013fb0d7c95278b0a2a3b7fa5bae Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 9 Sep 2012 21:41:29 +0200 Subject: [PATCH 19/23] [ticket/8713] Update untrimmed_variable() doc block. PHPBB3-8713 --- phpBB/includes/request/request.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/request/request.php b/phpBB/includes/request/request.php index aa62c3b610..a06fc0d85d 100644 --- a/phpBB/includes/request/request.php +++ b/phpBB/includes/request/request.php @@ -204,9 +204,9 @@ class phpbb_request implements phpbb_request_interface } /** - * Get a variable, but without trimming strings - * Same functionality as variable(), except does not run trim() on strings - * All variables in GET or POST requests should be retrieved through this function to maximise security. + * Get a variable, but without trimming strings. + * Same functionality as variable(), except does not run trim() on strings. + * This method should be used when handling passwords. * * @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 From 52b25aaf20f512911a981958e0efa9365eb34489 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 4 Oct 2012 01:51:57 +0530 Subject: [PATCH 20/23] [ticket/11051] add get_word_len() in sphinx search get_word_len() for sphinx search backend returns false always and triggers proper error. PHPBB3-11051 --- phpBB/includes/search/fulltext_sphinx.php | 10 ++++++++++ phpBB/search.php | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/search/fulltext_sphinx.php b/phpBB/includes/search/fulltext_sphinx.php index 256c3a6d1c..a6cc47b836 100644 --- a/phpBB/includes/search/fulltext_sphinx.php +++ b/phpBB/includes/search/fulltext_sphinx.php @@ -108,6 +108,16 @@ class phpbb_search_fulltext_sphinx return $this->search_query; } + /** + * Returns false as there is no word_len array + * + * @return false + */ + public function get_word_length() + { + return false; + } + /** * Returns the common_words array * diff --git a/phpBB/search.php b/phpBB/search.php index 54f0f225a0..e869c8eebd 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -296,7 +296,14 @@ if ($keywords || $author || $author_id || $search_id || $submit) $ignored = (sizeof($search->get_common_words())) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->get_common_words())) . '
' : ''; // Function cannot be directly used as associative array for php < 5.4 $word_length = $search->get_word_length(); - trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $word_length['min']), $user->lang('CHARACTERS', (int) $word_length['max']))); + if ($word_length) + { + trigger_error($ignored . $user->lang('NO_KEYWORDS', $user->lang('CHARACTERS', (int) $word_length['min']), $user->lang('CHARACTERS', (int) $word_length['max']))); + } + else + { + trigger_error($ignored); + } } } From 67939192ae287b958902b382c6dd344c2cb74a74 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 4 Oct 2012 23:17:18 +0530 Subject: [PATCH 21/23] [ticket/11051] remove unnecessary comment PHPBB3-11051 --- phpBB/search.php | 1 - 1 file changed, 1 deletion(-) diff --git a/phpBB/search.php b/phpBB/search.php index e869c8eebd..f392e386c7 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -294,7 +294,6 @@ if ($keywords || $author || $author_id || $search_id || $submit) if (!$correct_query || (!$search->get_search_query() && !sizeof($author_id_ary) && !$search_id)) { $ignored = (sizeof($search->get_common_words())) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->get_common_words())) . '
' : ''; - // Function cannot be directly used as associative array for php < 5.4 $word_length = $search->get_word_length(); if ($word_length) { From be884b7e290edfaf5db189e6c3cf22024906f159 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Thu, 4 Oct 2012 23:32:50 +0530 Subject: [PATCH 22/23] [ticket/11051] add common_words variable PHPBB3-11051 --- phpBB/search.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/phpBB/search.php b/phpBB/search.php index f392e386c7..7eda3c4d1d 100644 --- a/phpBB/search.php +++ b/phpBB/search.php @@ -287,13 +287,15 @@ if ($keywords || $author || $author_id || $search_id || $submit) trigger_error($error); } + $common_words = $search->get_common_words(); + // let the search module split up the keywords if ($keywords) { $correct_query = $search->split_keywords($keywords, $search_terms); if (!$correct_query || (!$search->get_search_query() && !sizeof($author_id_ary) && !$search_id)) { - $ignored = (sizeof($search->get_common_words())) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $search->get_common_words())) . '
' : ''; + $ignored = (sizeof($common_words)) ? sprintf($user->lang['IGNORED_TERMS_EXPLAIN'], implode(' ', $common_words)) . '
' : ''; $word_length = $search->get_word_length(); if ($word_length) { @@ -618,7 +620,7 @@ if ($keywords || $author || $author_id || $search_id || $submit) 'SEARCH_MATCHES' => $l_search_matches, 'SEARCH_WORDS' => $keywords, 'SEARCHED_QUERY' => $search->get_search_query(), - 'IGNORED_WORDS' => (sizeof($search->get_common_words())) ? implode(' ', $search->get_common_words()) : '', + 'IGNORED_WORDS' => (sizeof($common_words)) ? implode(' ', $common_words) : '', 'PAGE_NUMBER' => phpbb_on_page($template, $user, $u_search, $total_match_count, $per_page, $start), 'PHRASE_SEARCH_DISABLED' => $phrase_search_disabled, From 305abfde963e764d5e6be0c7b1c1b9496a2477b2 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Mon, 8 Oct 2012 10:58:04 +0530 Subject: [PATCH 23/23] [ticket/11051] fix spaces PHPBB3-11051 --- phpBB/includes/search/fulltext_native.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/search/fulltext_native.php b/phpBB/includes/search/fulltext_native.php index 1100484ebd..bbc2236b3c 100644 --- a/phpBB/includes/search/fulltext_native.php +++ b/phpBB/includes/search/fulltext_native.php @@ -23,9 +23,9 @@ if (!defined('IN_PHPBB')) class phpbb_search_fulltext_native extends phpbb_search_base { protected $stats = array(); - protected $word_length = array(); - protected $search_query; - protected $common_words = array(); + protected $word_length = array(); + protected $search_query; + protected $common_words = array(); protected $must_contain_ids = array(); protected $must_not_contain_ids = array();