aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNathaniel Guse <nathaniel.guse@gmail.com>2012-09-03 13:32:33 -0500
committerNathaniel Guse <nathaniel.guse@gmail.com>2012-09-03 13:32:33 -0500
commitb3cd5a649be62f175de651a16ae02c5f709ca2f4 (patch)
tree223329608a30291484735b729d4432c61b0362aa
parent7bf598954c452448c2807428f6517cd75d910f92 (diff)
downloadforums-b3cd5a649be62f175de651a16ae02c5f709ca2f4.tar
forums-b3cd5a649be62f175de651a16ae02c5f709ca2f4.tar.gz
forums-b3cd5a649be62f175de651a16ae02c5f709ca2f4.tar.bz2
forums-b3cd5a649be62f175de651a16ae02c5f709ca2f4.tar.xz
forums-b3cd5a649be62f175de651a16ae02c5f709ca2f4.zip
[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
-rw-r--r--phpBB/includes/acp/acp_language.php6
-rw-r--r--phpBB/includes/acp/acp_users.php6
-rw-r--r--phpBB/includes/functions.php4
-rw-r--r--phpBB/includes/request/request.php63
-rw-r--r--phpBB/includes/request/type_cast_helper.php22
-rw-r--r--phpBB/includes/ucp/ucp_profile.php6
-rw-r--r--phpBB/includes/ucp/ucp_register.php4
-rw-r--r--phpBB/install/install_update.php4
-rw-r--r--tests/request/type_cast_helper_test.php10
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
@@ -243,6 +243,69 @@ 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.
+ *
+ * @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.
*
* Also fall back to getenv(), some CGI setups may need it (probably not, but
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&lt;3 ');
+
+ $this->type_cast_helper->recursive_set_var($data, '', true, false);
+
+ $this->assertEquals($expected, $data);
+ }
}