diff options
author | Nils Adermann <naderman@naderman.de> | 2014-11-16 16:50:29 +0100 |
---|---|---|
committer | Nils Adermann <naderman@naderman.de> | 2014-11-16 16:50:29 +0100 |
commit | 6104be6b4d37f6ce569fde95c88400724dabac13 (patch) | |
tree | d92f43d438d5b6516a3538f826c81fe29522f2ab | |
parent | f1b88706b8930186c8a2dbe216769e2f8350af38 (diff) | |
parent | 4ffdb129388d72cdcc790f1c3713e3770d8aeca9 (diff) | |
download | forums-6104be6b4d37f6ce569fde95c88400724dabac13.tar forums-6104be6b4d37f6ce569fde95c88400724dabac13.tar.gz forums-6104be6b4d37f6ce569fde95c88400724dabac13.tar.bz2 forums-6104be6b4d37f6ce569fde95c88400724dabac13.tar.xz forums-6104be6b4d37f6ce569fde95c88400724dabac13.zip |
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"
-rw-r--r-- | phpBB/config/services.yml | 3 | ||||
-rw-r--r-- | phpBB/phpbb/controller/helper.php | 11 | ||||
-rw-r--r-- | phpBB/phpbb/path_helper.php | 6 | ||||
-rw-r--r-- | phpBB/phpbb/request/request.php | 23 | ||||
-rw-r--r-- | phpBB/phpbb/request/request_interface.php | 10 | ||||
-rw-r--r-- | phpBB/phpbb/session.php | 13 | ||||
-rw-r--r-- | phpBB/phpbb/symfony_request.php | 24 | ||||
-rw-r--r-- | tests/controller/common_helper_route.php | 32 | ||||
-rw-r--r-- | tests/functions/build_url_test.php | 1 | ||||
-rw-r--r-- | tests/mock/controller_helper.php | 3 | ||||
-rw-r--r-- | tests/mock/request.php | 21 | ||||
-rw-r--r-- | tests/pagination/pagination_test.php | 2 | ||||
-rw-r--r-- | tests/security/base.php | 50 | ||||
-rw-r--r-- | tests/security/extract_current_page_test.php | 61 | ||||
-rw-r--r-- | tests/security/redirect_test.php | 4 | ||||
-rw-r--r-- | tests/session/extract_page_test.php | 47 |
16 files changed, 164 insertions, 147 deletions
diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index f86850b746..6be694a14c 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -76,6 +76,7 @@ services: - @controller.provider - @ext.manager - @symfony_request + - @request - @filesystem - %core.root_path% - %core.php_ext% @@ -156,6 +157,8 @@ services: - null - %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: class: phpbb\symfony_request arguments: diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 187e455d48..52e6947c2c 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -44,6 +44,9 @@ class helper /* @var \phpbb\symfony_request */ protected $symfony_request; + /* @var \phpbb\request\request_interface */ + protected $request; + /** * @var \phpbb\filesystem The filesystem object */ @@ -70,16 +73,18 @@ class helper * @param \phpbb\controller\provider $provider Path provider * @param \phpbb\extension\manager $manager Extension manager 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 string $phpbb_root_path phpBB root path * @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->user = $user; $this->config = $config; $this->symfony_request = $symfony_request; + $this->request = $request; $this->filesystem = $filesystem; $this->phpbb_root_path = $phpbb_root_path; $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); @@ -197,6 +202,6 @@ class helper */ 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); } } diff --git a/phpBB/phpbb/path_helper.php b/phpBB/phpbb/path_helper.php index 936564d8b6..4a446a5d9d 100644 --- a/phpBB/phpbb/path_helper.php +++ b/phpBB/phpbb/path_helper.php @@ -154,6 +154,7 @@ class path_helper 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 = $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')) { + // 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( $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; } diff --git a/phpBB/phpbb/request/request.php b/phpBB/phpbb/request/request.php index ea9854894c..f0f2f7e2a2 100644 --- a/phpBB/phpbb/request/request.php +++ b/phpBB/phpbb/request/request.php @@ -416,4 +416,27 @@ class request implements \phpbb\request\request_interface { 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; + } } diff --git a/phpBB/phpbb/request/request_interface.php b/phpBB/phpbb/request/request_interface.php index 3236f73990..47b3b3a4ed 100644 --- a/phpBB/phpbb/request/request_interface.php +++ b/phpBB/phpbb/request/request_interface.php @@ -142,4 +142,14 @@ interface request_interface * @return array The original array of the requested super global. */ 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); } diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index a06ff9c594..dc90d942c3 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -31,10 +31,11 @@ class session var $update_session_page = true; /** - * Extract current session page - * - * @param string $root_path current root path (phpbb_root_path) - */ + * Extract current session page + * + * @param string $root_path current root path (phpbb_root_path) + * @return array + */ static function extract_current_page($root_path) { global $request, $symfony_request, $phpbb_filesystem; @@ -42,8 +43,8 @@ class session $page_array = array(); // First of all, get the request uri... - $script_name = $symfony_request->getScriptName(); - $args = explode('&', $symfony_request->getQueryString()); + $script_name = $request->escape($symfony_request->getScriptName(), true); + $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 (!$script_name) diff --git a/phpBB/phpbb/symfony_request.php b/phpBB/phpbb/symfony_request.php index 02d22c480f..2931cae3cc 100644 --- a/phpBB/phpbb/symfony_request.php +++ b/phpBB/phpbb/symfony_request.php @@ -15,6 +15,10 @@ namespace phpbb; 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 { /** @@ -24,32 +28,12 @@ class symfony_request extends 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); $post_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::POST); $server_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::SERVER); $files_parameters = $phpbb_request->get_super_global(\phpbb\request\request_interface::FILES); $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); } } diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index 1bde559fbb..5e15eb3ee1 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -59,21 +59,21 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case protected function generate_route_objects() { - $request = new phpbb_mock_request(); - $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); - $request->overwrite('REQUEST_URI', $this->get_uri(), \phpbb\request\request_interface::SERVER); - $request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER); - $request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER); + $this->request = new phpbb_mock_request(); + $this->request->overwrite('SCRIPT_NAME', $this->get_uri(), \phpbb\request\request_interface::SERVER); + $this->request->overwrite('SCRIPT_FILENAME', $this->get_script_name(), \phpbb\request\request_interface::SERVER); + $this->request->overwrite('REQUEST_URI', $this->get_uri(), \phpbb\request\request_interface::SERVER); + $this->request->overwrite('SERVER_NAME', 'localhost', \phpbb\request\request_interface::SERVER); + $this->request->overwrite('SERVER_PORT', '80', \phpbb\request\request_interface::SERVER); $this->symfony_request = new \phpbb\symfony_request( - $request + $this->request ); $this->filesystem = new \phpbb\filesystem(); $this->phpbb_path_helper = new \phpbb\path_helper( $this->symfony_request, $this->filesystem, - $this->getMock('\phpbb\request\request'), + $this->request, $phpbb_root_path, $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) { - $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)); } @@ -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) { $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)); } @@ -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) { $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)); } @@ -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) { $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)); } @@ -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) { $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)); } //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) { $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)); } @@ -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) { $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)); } @@ -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) { $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)); } } diff --git a/tests/functions/build_url_test.php b/tests/functions/build_url_test.php index 5cfd1300de..a59b94c744 100644 --- a/tests/functions/build_url_test.php +++ b/tests/functions/build_url_test.php @@ -85,7 +85,6 @@ class phpbb_build_url_test extends phpbb_test_case global $user, $phpbb_root_path; $user->page['page'] = $page; - $output = build_url($strip_vars); $this->assertEquals($expected, $output); diff --git a/tests/mock/controller_helper.php b/tests/mock/controller_helper.php index 9c13c309f2..ae3e7bf432 100644 --- a/tests/mock/controller_helper.php +++ b/tests/mock/controller_helper.php @@ -13,12 +13,13 @@ 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->user = $user; $this->config = $config; $this->symfony_request = $symfony_request; + $this->request = $request; $this->filesystem = $filesystem; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; diff --git a/tests/mock/request.php b/tests/mock/request.php index 304fcf0eaf..e7217a94a9 100644 --- a/tests/mock/request.php +++ b/tests/mock/request.php @@ -114,4 +114,25 @@ class phpbb_mock_request implements \phpbb\request\request_interface { $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; + } } diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index d36aa11a8a..494c667198 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -57,7 +57,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case $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); } diff --git a/tests/security/base.php b/tests/security/base.php index 5519cac441..330408b448 100644 --- a/tests/security/base.php +++ b/tests/security/base.php @@ -13,6 +13,8 @@ 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 */ @@ -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; // Put this into a global function being run by every test to init a proper user session - $server['HTTP_HOST'] = 'localhost'; - $server['SERVER_NAME'] = 'localhost'; - $server['SERVER_ADDR'] = '127.0.0.1'; - $server['SERVER_PORT'] = 80; - $server['REMOTE_ADDR'] = '127.0.0.1'; - $server['QUERY_STRING'] = ''; - $server['REQUEST_URI'] = '/tests/'; - $server['SCRIPT_NAME'] = '/tests/index.php'; - $server['PHP_SELF'] = '/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'; - $server['HTTP_ACCEPT_LANGUAGE'] = 'de-de,de;q=0.8,en-us;q=0.5,en;q=0.3'; + $this->server['HTTP_HOST'] = 'localhost'; + $this->server['SERVER_NAME'] = 'localhost'; + $this->server['SERVER_ADDR'] = '127.0.0.1'; + $this->server['SERVER_PORT'] = 80; + $this->server['REMOTE_ADDR'] = '127.0.0.1'; + $this->server['QUERY_STRING'] = ''; + $this->server['REQUEST_URI'] = '/tests/'; + $this->server['SCRIPT_NAME'] = '/tests/index.php'; + $this->server['SCRIPT_FILENAME'] = '/var/www/tests/index.php'; + $this->server['PHP_SELF'] = '/tests/index.php'; + $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 @@ -40,31 +43,18 @@ abstract class phpbb_security_test_base extends phpbb_test_case [SCRIPT_FILENAME] => /var/www/tests/index.php */ - $request = new phpbb_mock_request(array(), array(), array(), $server); - $symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( - $request, - )); - $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); + $request = new phpbb_mock_request(array(), array(), array(), $this->server); + $symfony_request = new \phpbb\symfony_request($request); + + $phpbb_filesystem = new \phpbb\filesystem(); // Set no user and trick a bit to circumvent errors $user = new \phpbb\user('\phpbb\datetime'); $user->lang = true; - $user->browser = $server['HTTP_USER_AGENT']; + $user->browser = $this->server['HTTP_USER_AGENT']; $user->referer = ''; $user->forwarded_for = ''; - $user->host = $server['HTTP_HOST']; + $user->host = $this->server['HTTP_HOST']; $user->page = \phpbb\session::extract_current_page($phpbb_root_path); } diff --git a/tests/security/extract_current_page_test.php b/tests/security/extract_current_page_test.php index c127b69b2b..767b901a43 100644 --- a/tests/security/extract_current_page_test.php +++ b/tests/security/extract_current_page_test.php @@ -20,33 +20,25 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base public function security_variables() { 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('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="><script>alert(/XSS/);</script>', 'mark=forums&x=%22%3E%3Cscript%3Ealert%28%2FXSS%2F%29%3B%3C%2Fscript%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 */ - 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; - $symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( - $request, - )); - $symfony_request->expects($this->any()) - ->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($server['REQUEST_URI'])); - $symfony_request->expects($this->sanitizer($this->any())) - ->method('getPathInfo') - ->will($this->returnValue($this->sanitizer('/'))); + $this->server['REQUEST_URI'] = ''; + $this->server['QUERY_STRING'] = $query_string; + + $request = new phpbb_mock_request(array(), array(), array(), $this->server); + $symfony_request = new \phpbb\symfony_request($request); + $result = \phpbb\session::extract_current_page('./'); $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 */ - 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; - $symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( - $request, - )); - $symfony_request->expects($this->any()) - ->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('/'))); + $this->server['QUERY_STRING'] = $query_string; + + $request = new phpbb_mock_request(array(), array(), array(), $this->server); + $symfony_request = new \phpbb\symfony_request($request); $result = \phpbb\session::extract_current_page('./'); $label = 'Running extract_current_page on ' . $query_string . ' with REQUEST_URI filled.'; $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('&', '&', $value); - } } diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php index 3961c2781e..21fb103ed1 100644 --- a/tests/security/redirect_test.php +++ b/tests/security/redirect_test.php @@ -73,6 +73,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base protected function setUp() { + global $phpbb_dispatcher; + parent::setUp(); $GLOBALS['config'] = array( @@ -80,6 +82,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base ); $this->path_helper = $this->get_path_helper(); + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); } /** diff --git a/tests/session/extract_page_test.php b/tests/session/extract_page_test.php index f314d35f87..f0d1cdb60e 100644 --- a/tests/session/extract_page_test.php +++ b/tests/session/extract_page_test.php @@ -12,6 +12,7 @@ */ 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 { @@ -99,7 +100,7 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case // ^-- Ignored because .. returns different directory in live vs testing 'query_string' => '', 'script_path' => '/phpBB/adm/', - //'root_script_path' => '/phpBB/', + //'root_script_path' => '/phpBB/adm/', //'page' => 'adm/index.php', 'forum' => 0, ), @@ -108,15 +109,15 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case './', '/phpBB/adm/app.php', 'page=1&test=2', - '/phpBB/', + '/phpBB/adm/', '/foo/bar', array( 'page_name' => 'app.php/foo/bar', - 'page_dir' => '', + //'page_dir' => '', 'query_string' => 'page=1&test=2', - 'script_path' => '/phpBB/', - 'root_script_path' => '/phpBB/', - 'page' => 'app.php/foo/bar?page=1&test=2', + 'script_path' => '/phpBB/adm/', + //'root_script_path' => '/phpBB/adm/', + //'page' => 'app.php/foo/bar?page=1&test=2', 'forum' => 0, ), ), @@ -142,23 +143,25 @@ class phpbb_session_extract_page_test extends phpbb_session_test_case /** @dataProvider extract_current_page_data */ function test_extract_current_page($root_path, $getScriptName, $getQueryString, $getBasePath, $getPathInfo, $expected) { - global $symfony_request; + global $symfony_request, $request, $phpbb_filesystem; + + $phpbb_filesystem = new \phpbb\filesystem(); + + $server['HTTP_HOST'] = 'localhost'; + $server['SERVER_NAME'] = 'localhost'; + $server['SERVER_ADDR'] = '127.0.0.1'; + $server['SERVER_PORT'] = 80; + $server['REMOTE_ADDR'] = '127.0.0.1'; + $server['QUERY_STRING'] = $getQueryString; + $server['REQUEST_URI'] = $getScriptName . $getPathInfo . ($getQueryString === '' ? '' : '?' . $getQueryString); + $server['SCRIPT_NAME'] = $getScriptName; + $server['SCRIPT_FILENAME'] = '/var/www/' . $getScriptName; + $server['PHP_SELF'] = $getScriptName; + $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'; + $server['HTTP_ACCEPT_LANGUAGE'] = 'de-de,de;q=0.8,en-us;q=0.5,en;q=0.3'; - $symfony_request = $this->getMock("\phpbb\symfony_request", array(), array( - new phpbb_mock_request(), - )); - $symfony_request->expects($this->any()) - ->method('getScriptName') - ->will($this->returnValue($getScriptName)); - $symfony_request->expects($this->any()) - ->method('getQueryString') - ->will($this->returnValue($getQueryString)); - $symfony_request->expects($this->any()) - ->method('getBasePath') - ->will($this->returnValue($getBasePath)); - $symfony_request->expects($this->any()) - ->method('getPathInfo') - ->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); |