Merge branch 'develop-ascraeus' into develop

* develop-ascraeus:
  [ticket/13280] Output escaping for the symfony request object
  [ticket/13280] Add new tests
  [ticket/13280] Make the tests failing
  [ticket/13280] Revert "Merge pull request #3107 from marc1706/ticket/13280"
This commit is contained in:
Nils Adermann 2014-11-16 16:50:29 +01:00
commit 6104be6b4d
16 changed files with 164 additions and 147 deletions

View file

@ -76,6 +76,7 @@ services:
- @controller.provider - @controller.provider
- @ext.manager - @ext.manager
- @symfony_request - @symfony_request
- @request
- @filesystem - @filesystem
- %core.root_path% - %core.root_path%
- %core.php_ext% - %core.php_ext%
@ -156,6 +157,8 @@ services:
- null - null
- %core.disable_super_globals% - %core.disable_super_globals%
# WARNING: The Symfony request does not escape the input and should be used very carefully
# prefer the phpbb request (service @request) as possible
symfony_request: symfony_request:
class: phpbb\symfony_request class: phpbb\symfony_request
arguments: arguments:

View file

@ -44,6 +44,9 @@ class helper
/* @var \phpbb\symfony_request */ /* @var \phpbb\symfony_request */
protected $symfony_request; protected $symfony_request;
/* @var \phpbb\request\request_interface */
protected $request;
/** /**
* @var \phpbb\filesystem The filesystem object * @var \phpbb\filesystem The filesystem object
*/ */
@ -70,16 +73,18 @@ class helper
* @param \phpbb\controller\provider $provider Path provider * @param \phpbb\controller\provider $provider Path provider
* @param \phpbb\extension\manager $manager Extension manager object * @param \phpbb\extension\manager $manager Extension manager object
* @param \phpbb\symfony_request $symfony_request Symfony Request object * @param \phpbb\symfony_request $symfony_request Symfony Request object
* @param \phpbb\request\request_interface $request phpBB request object
* @param \phpbb\filesystem $filesystem The filesystem object * @param \phpbb\filesystem $filesystem The filesystem object
* @param string $phpbb_root_path phpBB root path * @param string $phpbb_root_path phpBB root path
* @param string $php_ext PHP file extension * @param string $php_ext PHP file extension
*/ */
public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, \phpbb\symfony_request $symfony_request, \phpbb\filesystem $filesystem, $phpbb_root_path, $php_ext) public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, \phpbb\symfony_request $symfony_request, \phpbb\request\request_interface $request, \phpbb\filesystem $filesystem, $phpbb_root_path, $php_ext)
{ {
$this->template = $template; $this->template = $template;
$this->user = $user; $this->user = $user;
$this->config = $config; $this->config = $config;
$this->symfony_request = $symfony_request; $this->symfony_request = $symfony_request;
$this->request = $request;
$this->filesystem = $filesystem; $this->filesystem = $filesystem;
$this->phpbb_root_path = $phpbb_root_path; $this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $php_ext; $this->php_ext = $php_ext;
@ -153,7 +158,7 @@ class helper
} }
} }
$base_url = $this->filesystem->clean_path($base_url); $base_url = $this->request->escape($this->filesystem->clean_path($base_url), true);
$context->setBaseUrl($base_url); $context->setBaseUrl($base_url);
@ -197,6 +202,6 @@ class helper
*/ */
public function get_current_url() public function get_current_url()
{ {
return generate_board_url(true) . $this->symfony_request->getRequestUri(); return generate_board_url(true) . $this->request->escape($this->symfony_request->getRequestUri(), true);
} }
} }

View file

@ -154,6 +154,7 @@ class path_helper
return $this->web_root_path; return $this->web_root_path;
} }
// We do not need to escape $path_info, $request_uri and $script_name because we can not find their content in the result.
// Path info (e.g. /foo/bar) // Path info (e.g. /foo/bar)
$path_info = $this->filesystem->clean_path($this->symfony_request->getPathInfo()); $path_info = $this->filesystem->clean_path($this->symfony_request->getPathInfo());
@ -203,9 +204,12 @@ class path_helper
*/ */
if ($this->request->is_ajax() && $this->symfony_request->get('_referer')) if ($this->request->is_ajax() && $this->symfony_request->get('_referer'))
{ {
// We need to escape $absolute_board_url because it can be partially concatenated to the result.
$absolute_board_url = $this->request->escape($this->symfony_request->getSchemeAndHttpHost() . $this->symfony_request->getBasePath(), true);
$referer_web_root_path = $this->get_web_root_path_from_ajax_referer( $referer_web_root_path = $this->get_web_root_path_from_ajax_referer(
$this->symfony_request->get('_referer'), $this->symfony_request->get('_referer'),
$this->symfony_request->getSchemeAndHttpHost() . $this->symfony_request->getBasePath() $absolute_board_url
); );
return $this->web_root_path = $this->phpbb_root_path . $referer_web_root_path; return $this->web_root_path = $this->phpbb_root_path . $referer_web_root_path;
} }

View file

@ -416,4 +416,27 @@ class request implements \phpbb\request\request_interface
{ {
return $this->input[$super_global]; return $this->input[$super_global];
} }
/**
* {@inheritdoc}
*/
public function escape($var, $multibyte)
{
if (is_array($var))
{
$result = array();
foreach ($var as $key => $value)
{
$this->type_cast_helper->set_var($key, $key, gettype($key), $multibyte);
$result[$key] = $this->escape($value, $multibyte);
}
$var = $result;
}
else
{
$this->type_cast_helper->set_var($var, $var, 'string', $multibyte);
}
return $var;
}
} }

View file

@ -142,4 +142,14 @@ interface request_interface
* @return array The original array of the requested super global. * @return array The original array of the requested super global.
*/ */
public function get_super_global($super_global = \phpbb\request\request_interface::REQUEST); public function get_super_global($super_global = \phpbb\request\request_interface::REQUEST);
/**
* Escape a string variable.
*
* @param mixed $value The contents to fill with
* @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.
* @return string|array
*/
public function escape($value, $multibyte);
} }

View file

@ -31,10 +31,11 @@ class session
var $update_session_page = true; var $update_session_page = true;
/** /**
* Extract current session page * Extract current session page
* *
* @param string $root_path current root path (phpbb_root_path) * @param string $root_path current root path (phpbb_root_path)
*/ * @return array
*/
static function extract_current_page($root_path) static function extract_current_page($root_path)
{ {
global $request, $symfony_request, $phpbb_filesystem; global $request, $symfony_request, $phpbb_filesystem;
@ -42,8 +43,8 @@ class session
$page_array = array(); $page_array = array();
// First of all, get the request uri... // First of all, get the request uri...
$script_name = $symfony_request->getScriptName(); $script_name = $request->escape($symfony_request->getScriptName(), true);
$args = explode('&', $symfony_request->getQueryString()); $args = $request->escape(explode('&', $symfony_request->getQueryString()), true);
// If we are unable to get the script name we use REQUEST_URI as a failover and note it within the page array for easier support... // If we are unable to get the script name we use REQUEST_URI as a failover and note it within the page array for easier support...
if (!$script_name) if (!$script_name)

View file

@ -15,6 +15,10 @@ namespace phpbb;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
/**
* WARNING: The Symfony request does not escape the input and should be used very carefully
* prefer the phpbb request as possible
*/
class symfony_request extends Request class symfony_request extends Request
{ {
/** /**
@ -24,32 +28,12 @@ class symfony_request extends Request
*/ */
public function __construct(\phpbb\request\request_interface $phpbb_request) public function __construct(\phpbb\request\request_interface $phpbb_request)
{ {
// This function is meant to sanitize the global input arrays
$sanitizer = function(&$value, $key) {
$type_cast_helper = new \phpbb\request\type_cast_helper();
$type_cast_helper->set_var($value, $value, gettype($value), true);
};
// This function is meant for additional handling of server variables
$server_sanitizer = function(&$value, $key) use ($sanitizer) {
$sanitizer($value, $key);
$value = str_replace('&', '&', $value);
};
$get_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::GET); $get_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::GET);
$post_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::POST); $post_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::POST);
$server_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::SERVER); $server_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::SERVER);
$files_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::FILES); $files_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::FILES);
$cookie_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::COOKIE); $cookie_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::COOKIE);
array_walk_recursive($get_parameters, $sanitizer);
array_walk_recursive($post_parameters, $sanitizer);
array_walk_recursive($files_parameters, $sanitizer);
array_walk_recursive($cookie_parameters, $sanitizer);
// Run special sanitizer for server superglobal
array_walk_recursive($server_parameters, $server_sanitizer);
parent::__construct($get_parameters, $post_parameters, array(), $cookie_parameters, $files_parameters, $server_parameters); parent::__construct($get_parameters, $post_parameters, array(), $cookie_parameters, $files_parameters, $server_parameters);
} }
} }

View file

@ -59,21 +59,21 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
protected function generate_route_objects() protected function generate_route_objects()
{ {
$request = new phpbb_mock_request(); $this->request = new phpbb_mock_request();
$request->overwrite('SCRIPT_NAME', $this->get_uri(), \phpbb\request\request_interface::SERVER); $this->request->overwrite('SCRIPT_NAME', $this->get_uri(), \phpbb\request\request_interface::SERVER);
$request->overwrite('SCRIPT_FILENAME', $this->get_script_name(), \phpbb\request\request_interface::SERVER); $this->request->overwrite('SCRIPT_FILENAME', $this->get_script_name(), \phpbb\request\request_interface::SERVER);
$request->overwrite('REQUEST_URI', $this->get_uri(), \phpbb\request\request_interface::SERVER); $this->request->overwrite('REQUEST_URI', $this->get_uri(), \phpbb\request\request_interface::SERVER);
$request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER); $this->request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER);
$request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER); $this->request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER);
$this->symfony_request = new \phpbb\symfony_request( $this->symfony_request = new \phpbb\symfony_request(
$request $this->request
); );
$this->filesystem = new \phpbb\filesystem(); $this->filesystem = new \phpbb\filesystem();
$this->phpbb_path_helper = new \phpbb\path_helper( $this->phpbb_path_helper = new \phpbb\path_helper(
$this->symfony_request, $this->symfony_request,
$this->filesystem, $this->filesystem,
$this->getMock('\phpbb\request\request'), $this->request,
$phpbb_root_path, $phpbb_root_path,
$phpEx $phpEx
); );
@ -161,7 +161,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
*/ */
public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
} }
@ -201,7 +201,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
} }
@ -241,7 +241,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_absolute($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_absolute($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL));
} }
@ -281,7 +281,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_relative_path($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_relative_path($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH));
} }
@ -321,7 +321,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_network($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_network($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH));
} }
//TODO //TODO
@ -361,7 +361,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_absolute_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_absolute_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL));
} }
@ -401,7 +401,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_relative_path_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_relative_path_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH));
} }
@ -441,7 +441,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
public function test_helper_url_network_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) public function test_helper_url_network_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
{ {
$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $this->provider, $this->extension_manager, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php', dirname(__FILE__) . '/');
$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH)); $this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH));
} }
} }

View file

@ -85,7 +85,6 @@ class phpbb_build_url_test extends phpbb_test_case
global $user, $phpbb_root_path; global $user, $phpbb_root_path;
$user->page['page'] = $page; $user->page['page'] = $page;
$output = build_url($strip_vars); $output = build_url($strip_vars);
$this->assertEquals($expected, $output); $this->assertEquals($expected, $output);

View file

@ -13,12 +13,13 @@
class phpbb_mock_controller_helper extends \phpbb\controller\helper class phpbb_mock_controller_helper extends \phpbb\controller\helper
{ {
public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, \phpbb\symfony_request $symfony_request, \phpbb\filesystem $filesystem, $phpbb_root_path, $php_ext, $phpbb_root_path_ext) public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, \phpbb\extension\manager $manager, \phpbb\symfony_request $symfony_request, \phpbb\request\request_interface $request, \phpbb\filesystem $filesystem, $phpbb_root_path, $php_ext, $phpbb_root_path_ext)
{ {
$this->template = $template; $this->template = $template;
$this->user = $user; $this->user = $user;
$this->config = $config; $this->config = $config;
$this->symfony_request = $symfony_request; $this->symfony_request = $symfony_request;
$this->request = $request;
$this->filesystem = $filesystem; $this->filesystem = $filesystem;
$this->phpbb_root_path = $phpbb_root_path; $this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $php_ext; $this->php_ext = $php_ext;

View file

@ -114,4 +114,25 @@ class phpbb_mock_request implements \phpbb\request\request_interface
{ {
$this->data[$super_global] = array_merge($this->data[$super_global], $values); $this->data[$super_global] = array_merge($this->data[$super_global], $values);
} }
public function escape($var, $multibyte)
{
$type_cast_helper = new \phpbb\request\type_cast_helper();
if (is_array($var))
{
$result = array();
foreach ($var as $key => $value)
{
$type_cast_helper->set_var($key, $key, gettype($key), $multibyte);
$result[$key] = $this->escape($value, $multibyte);
}
$var = $result;
}
else
{
$type_cast_helper->set_var($var, $var, 'string', $multibyte);
}
return $var;
}
} }

View file

@ -57,7 +57,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
$request $request
); );
$this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $provider, $manager, $symfony_request, $filesystem, '', 'php', dirname(__FILE__) . '/'); $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $provider, $manager, $symfony_request, $request, $filesystem, '', 'php', dirname(__FILE__) . '/');
$this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper, $phpbb_dispatcher); $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper, $phpbb_dispatcher);
} }

View file

@ -13,6 +13,8 @@
abstract class phpbb_security_test_base extends phpbb_test_case abstract class phpbb_security_test_base extends phpbb_test_case
{ {
protected $server = array();
/** /**
* Set up the required user object and server variables for the suites * Set up the required user object and server variables for the suites
*/ */
@ -21,17 +23,18 @@ abstract class phpbb_security_test_base extends phpbb_test_case
global $user, $phpbb_root_path, $phpEx, $request, $symfony_request, $phpbb_filesystem; global $user, $phpbb_root_path, $phpEx, $request, $symfony_request, $phpbb_filesystem;
// Put this into a global function being run by every test to init a proper user session // Put this into a global function being run by every test to init a proper user session
$server['HTTP_HOST'] = 'localhost'; $this->server['HTTP_HOST'] = 'localhost';
$server['SERVER_NAME'] = 'localhost'; $this->server['SERVER_NAME'] = 'localhost';
$server['SERVER_ADDR'] = '127.0.0.1'; $this->server['SERVER_ADDR'] = '127.0.0.1';
$server['SERVER_PORT'] = 80; $this->server['SERVER_PORT'] = 80;
$server['REMOTE_ADDR'] = '127.0.0.1'; $this->server['REMOTE_ADDR'] = '127.0.0.1';
$server['QUERY_STRING'] = ''; $this->server['QUERY_STRING'] = '';
$server['REQUEST_URI'] = '/tests/'; $this->server['REQUEST_URI'] = '/tests/';
$server['SCRIPT_NAME'] = '/tests/index.php'; $this->server['SCRIPT_NAME'] = '/tests/index.php';
$server['PHP_SELF'] = '/tests/index.php'; $this->server['SCRIPT_FILENAME'] = '/var/www/tests/index.php';
$server['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14'; $this->server['PHP_SELF'] = '/tests/index.php';
$server['HTTP_ACCEPT_LANGUAGE'] = 'de-de,de;q=0.8,en-us;q=0.5,en;q=0.3'; $this->server['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14';
$this->server['HTTP_ACCEPT_LANGUAGE'] = 'de-de,de;q=0.8,en-us;q=0.5,en;q=0.3';
/* /*
[HTTP_ACCEPT_ENCODING] => gzip,deflate [HTTP_ACCEPT_ENCODING] => gzip,deflate
@ -40,31 +43,18 @@ abstract class phpbb_security_test_base extends phpbb_test_case
[SCRIPT_FILENAME] => /var/www/tests/index.php [SCRIPT_FILENAME] => /var/www/tests/index.php
*/ */
$request = new phpbb_mock_request(array(), array(), array(), $server); $request = new phpbb_mock_request(array(), array(), array(), $this->server);
$symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( $symfony_request = new \phpbb\symfony_request($request);
$request,
)); $phpbb_filesystem = new \phpbb\filesystem();
$symfony_request->expects($this->any())
->method('getScriptName')
->will($this->returnValue($server['SCRIPT_NAME']));
$symfony_request->expects($this->any())
->method('getQueryString')
->will($this->returnValue($server['QUERY_STRING']));
$symfony_request->expects($this->any())
->method('getBasePath')
->will($this->returnValue($server['REQUEST_URI']));
$symfony_request->expects($this->any())
->method('getPathInfo')
->will($this->returnValue('/'));
$phpbb_filesystem = new \phpbb\filesystem($symfony_request, $phpbb_root_path, $phpEx);
// Set no user and trick a bit to circumvent errors // Set no user and trick a bit to circumvent errors
$user = new \phpbb\user('\phpbb\datetime'); $user = new \phpbb\user('\phpbb\datetime');
$user->lang = true; $user->lang = true;
$user->browser = $server['HTTP_USER_AGENT']; $user->browser = $this->server['HTTP_USER_AGENT'];
$user->referer = ''; $user->referer = '';
$user->forwarded_for = ''; $user->forwarded_for = '';
$user->host = $server['HTTP_HOST']; $user->host = $this->server['HTTP_HOST'];
$user->page = \phpbb\session::extract_current_page($phpbb_root_path); $user->page = \phpbb\session::extract_current_page($phpbb_root_path);
} }

View file

@ -20,33 +20,25 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base
public function security_variables() public function security_variables()
{ {
return array( return array(
array('http://localhost/phpBB/index.php', 'mark=forums&x="><script>alert(/XSS/);</script>', 'mark=forums&x=%22%3E%3Cscript%3Ealert(/XSS/);%3C/script%3E'), array('mark=forums&x="><script>alert(/XSS/);</script>', 'mark=forums&x=%22%3E%3Cscript%3Ealert%28%2FXSS%2F%29%3B%3C%2Fscript%3E'),
array('http://localhost/phpBB/index.php', 'mark=forums&x=%22%3E%3Cscript%3Ealert(/XSS/);%3C/script%3E', 'mark=forums&x=%22%3E%3Cscript%3Ealert(/XSS/);%3C/script%3E'), array('mark=forums&x=%22%3E%3Cscript%3Ealert(/XSS/);%3C/script%3E', 'mark=forums&x=%22%3E%3Cscript%3Ealert%28%2FXSS%2F%29%3B%3C%2Fscript%3E'),
array('mark=forums&x=%22%3E%3Cscript%3Ealert%28%2FXSS%2F%29%3B%3C%2Fscript%3E', 'mark=forums&x=%22%3E%3Cscript%3Ealert%28%2FXSS%2F%29%3B%3C%2Fscript%3E'),
); );
} }
/** /**
* @dataProvider security_variables * @dataProvider security_variables
*/ */
public function test_query_string_php_self($url, $query_string, $expected) public function test_query_string_php_self($query_string, $expected)
{ {
global $symfony_request, $request; global $symfony_request, $request;
$symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( $this->server['REQUEST_URI'] = '';
$request, $this->server['QUERY_STRING'] = $query_string;
));
$symfony_request->expects($this->any()) $request = new phpbb_mock_request(array(), array(), array(), $this->server);
->method('getScriptName') $symfony_request = new \phpbb\symfony_request($request);
->will($this->returnValue($this->sanitizer($url)));
$symfony_request->expects($this->any())
->method('getQueryString')
->will($this->returnValue($this->sanitizer($query_string)));
$symfony_request->expects($this->any())
->method('getBasePath')
->will($this->returnValue($server['REQUEST_URI']));
$symfony_request->expects($this->sanitizer($this->any()))
->method('getPathInfo')
->will($this->returnValue($this->sanitizer('/')));
$result = \phpbb\session::extract_current_page('./'); $result = \phpbb\session::extract_current_page('./');
$label = 'Running extract_current_page on ' . $query_string . ' with PHP_SELF filled.'; $label = 'Running extract_current_page on ' . $query_string . ' with PHP_SELF filled.';
@ -56,41 +48,18 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base
/** /**
* @dataProvider security_variables * @dataProvider security_variables
*/ */
public function test_query_string_request_uri($url, $query_string, $expected) public function test_query_string_request_uri($query_string, $expected)
{ {
global $symfony_request, $request; global $symfony_request, $request;
$symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( $this->server['QUERY_STRING'] = $query_string;
$request,
)); $request = new phpbb_mock_request(array(), array(), array(), $this->server);
$symfony_request->expects($this->any()) $symfony_request = new \phpbb\symfony_request($request);
->method('getScriptName')
->will($this->returnValue($this->sanitizer($url)));
$symfony_request->expects($this->any())
->method('getQueryString')
->will($this->returnValue($this->sanitizer($query_string)));
$symfony_request->expects($this->any())
->method('getBasePath')
->will($this->returnValue($this->sanitizer($server['REQUEST_URI'])));
$symfony_request->expects($this->any())
->method('getPathInfo')
->will($this->returnValue($this->sanitizer('/')));
$result = \phpbb\session::extract_current_page('./'); $result = \phpbb\session::extract_current_page('./');
$label = 'Running extract_current_page on ' . $query_string . ' with REQUEST_URI filled.'; $label = 'Running extract_current_page on ' . $query_string . ' with REQUEST_URI filled.';
$this->assertEquals($expected, $result['query_string'], $label); $this->assertEquals($expected, $result['query_string'], $label);
} }
protected function sanitizer($value)
{
// Fix for objects passed in phpunit
if (is_object($value))
{
return $value;
}
$type_cast_helper = new \phpbb\request\type_cast_helper();
$type_cast_helper->set_var($value, $value, gettype($value), true);
return str_replace('&amp;', '&', $value);
}
} }

View file

@ -73,6 +73,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
protected function setUp() protected function setUp()
{ {
global $phpbb_dispatcher;
parent::setUp(); parent::setUp();
$GLOBALS['config'] = array( $GLOBALS['config'] = array(
@ -80,6 +82,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
); );
$this->path_helper = $this->get_path_helper(); $this->path_helper = $this->get_path_helper();
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
} }
/** /**

View file

@ -12,6 +12,7 @@
*/ */
require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php'; require_once dirname(__FILE__) . '/../test_framework/phpbb_session_test_case.php';
require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
class phpbb_session_extract_page_test extends phpbb_session_test_case class phpbb_session_extract_page_test extends phpbb_session_test_case
{ {
@ -99,7 +100,7 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
// ^-- Ignored because .. returns different directory in live vs testing // ^-- Ignored because .. returns different directory in live vs testing
'query_string' => '', 'query_string' => '',
'script_path' => '/phpBB/adm/', 'script_path' => '/phpBB/adm/',
//'root_script_path' => '/phpBB/', //'root_script_path' => '/phpBB/adm/',
//'page' => 'adm/index.php', //'page' => 'adm/index.php',
'forum' => 0, 'forum' => 0,
), ),
@ -108,15 +109,15 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
'./', './',
'/phpBB/adm/app.php', '/phpBB/adm/app.php',
'page=1&test=2', 'page=1&test=2',
'/phpBB/', '/phpBB/adm/',
'/foo/bar', '/foo/bar',
array( array(
'page_name' => 'app.php/foo/bar', 'page_name' => 'app.php/foo/bar',
'page_dir' => '', //'page_dir' => '',
'query_string' => 'page=1&test=2', 'query_string' => 'page=1&test=2',
'script_path' => '/phpBB/', 'script_path' => '/phpBB/adm/',
'root_script_path' => '/phpBB/', //'root_script_path' => '/phpBB/adm/',
'page' => 'app.php/foo/bar?page=1&test=2', //'page' => 'app.php/foo/bar?page=1&test=2',
'forum' => 0, 'forum' => 0,
), ),
), ),
@ -142,23 +143,25 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case
/** @dataProvider extract_current_page_data */ /** @dataProvider extract_current_page_data */
function test_extract_current_page($root_path, $getScriptName, $getQueryString, $getBasePath, $getPathInfo, $expected) function test_extract_current_page($root_path, $getScriptName, $getQueryString, $getBasePath, $getPathInfo, $expected)
{ {
global $symfony_request; global $symfony_request, $request, $phpbb_filesystem;
$symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( $phpbb_filesystem = new \phpbb\filesystem();
new phpbb_mock_request(),
)); $server['HTTP_HOST'] = 'localhost';
$symfony_request->expects($this->any()) $server['SERVER_NAME'] = 'localhost';
->method('getScriptName') $server['SERVER_ADDR'] = '127.0.0.1';
->will($this->returnValue($getScriptName)); $server['SERVER_PORT'] = 80;
$symfony_request->expects($this->any()) $server['REMOTE_ADDR'] = '127.0.0.1';
->method('getQueryString') $server['QUERY_STRING'] = $getQueryString;
->will($this->returnValue($getQueryString)); $server['REQUEST_URI'] = $getScriptName . $getPathInfo . ($getQueryString === '' ? '' : '?' . $getQueryString);
$symfony_request->expects($this->any()) $server['SCRIPT_NAME'] = $getScriptName;
->method('getBasePath') $server['SCRIPT_FILENAME'] = '/var/www/' . $getScriptName;
->will($this->returnValue($getBasePath)); $server['PHP_SELF'] = $getScriptName;
$symfony_request->expects($this->any()) $server['HTTP_USER_AGENT'] = 'Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14';
->method('getPathInfo') $server['HTTP_ACCEPT_LANGUAGE'] = 'de-de,de;q=0.8,en-us;q=0.5,en;q=0.3';
->will($this->returnValue($getPathInfo));
$request = new phpbb_mock_request(array(), array(), array(), $server);
$symfony_request = new \phpbb\symfony_request($request);
$output = \phpbb\session::extract_current_page($root_path); $output = \phpbb\session::extract_current_page($root_path);