aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarc Alexander <admin@m-a-styles.de>2013-12-27 13:00:22 +0100
committerMarc Alexander <admin@m-a-styles.de>2013-12-27 13:00:22 +0100
commit9161816267ace60d9972e01b7fbe1213fe29a708 (patch)
treedce184bb1124f051bded43a0f056715249729cf5
parent4ced1626467c1b8a137f18ceb3aca3a032b41c76 (diff)
downloadforums-9161816267ace60d9972e01b7fbe1213fe29a708.tar
forums-9161816267ace60d9972e01b7fbe1213fe29a708.tar.gz
forums-9161816267ace60d9972e01b7fbe1213fe29a708.tar.bz2
forums-9161816267ace60d9972e01b7fbe1213fe29a708.tar.xz
forums-9161816267ace60d9972e01b7fbe1213fe29a708.zip
[ticket/11997] Do not check if file or dir we redirect to exist
The redirect function will now properly redirect to where we want it to. It will no longer try to check if the file or directory we redirect to exist. This will ensure compatibility with the new routes. PHPBB3-11997
Diffstat
-rw-r--r--phpBB/includes/functions.php88
-rw-r--r--tests/security/redirect_test.php70
2 files changed, 69 insertions, 89 deletions
diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php
index aea13f7679..33d0e36a28 100644
--- a/phpBB/includes/functions.php
+++ b/phpBB/includes/functions.php
@@ -2696,89 +2696,27 @@ function redirect($url, $return = false, $disable_cd_check = false)
// Relative uri
$pathinfo = pathinfo($url);
- // Also treat URLs that have a non-existing basename and fit
- // controller style URLs
- if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url))))
+ // Is the uri pointing to the current directory?
+ if ($pathinfo['dirname'] == '.')
{
- $url = str_replace('../', '', $url);
- $pathinfo = pathinfo($url);
+ $url = str_replace('./', '', $url);
- // Also treat URLs that have a non-existing basename
- if (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url)))
+ // Strip / from the beginning
+ if ($url && substr($url, 0, 1) == '/')
{
- // fallback to "last known user page"
- // at least this way we know the user does not leave the phpBB root
- if ($phpbb_path_helper instanceof \phpbb\path_helper)
- {
- $url = $phpbb_path_helper->get_controller_redirect_url($url);
- }
- else
- {
- $url = generate_board_url() . '/' . $user->page['page'];
- }
- $failover_flag = true;
+ $url = substr($url, 1);
}
}
- if (!$failover_flag)
- {
- // Is the uri pointing to the current directory?
- if ($pathinfo['dirname'] == '.')
- {
- $url = str_replace('./', '', $url);
-
- // Strip / from the beginning
- if ($url && substr($url, 0, 1) == '/')
- {
- $url = substr($url, 1);
- }
-
- if ($user->page['page_dir'])
- {
- $url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url;
- }
- else
- {
- $url = generate_board_url() . '/' . $url;
- }
- }
- else
- {
- // Used ./ before, but $phpbb_root_path is working better with urls within another root path
- $root_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($phpbb_root_path)));
- $page_dirs = explode('/', str_replace('\\', '/', phpbb_realpath($pathinfo['dirname'])));
- $intersection = array_intersect_assoc($root_dirs, $page_dirs);
-
- $root_dirs = array_diff_assoc($root_dirs, $intersection);
- $page_dirs = array_diff_assoc($page_dirs, $intersection);
-
- $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
-
- // Strip / from the end
- if ($dir && substr($dir, -1, 1) == '/')
- {
- $dir = substr($dir, 0, -1);
- }
-
- // Strip / from the beginning
- if ($dir && substr($dir, 0, 1) == '/')
- {
- $dir = substr($dir, 1);
- }
-
- $url = str_replace($pathinfo['dirname'] . '/', '', $url);
+ $url = generate_board_url() . '/' . $url;
+ }
- // Strip / from the beginning
- if (substr($url, 0, 1) == '/')
- {
- $url = substr($url, 1);
- }
+ // Clean URL and check if we go outside the forum directory
+ $url = $phpbb_path_helper->clean_url($url);
- $url = (!empty($dir) ? $dir . '/' : '') . $url;
- $url = generate_board_url() . '/' . $url;
- }
- $url = $phpbb_path_helper->clean_url($url);;
- }
+ if (!$disable_cd_check && strpos($url, generate_board_url(true)) === false)
+ {
+ trigger_error('INSECURE_REDIRECT', E_USER_ERROR);
}
// Make sure no linebreaks are there... to prevent http response splitting for PHP < 4.4.2
diff --git a/tests/security/redirect_test.php b/tests/security/redirect_test.php
index 13db4c299b..ce2d865c44 100644
--- a/tests/security/redirect_test.php
+++ b/tests/security/redirect_test.php
@@ -13,8 +13,13 @@ require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
class phpbb_security_redirect_test extends phpbb_security_test_base
{
+ protected $path_helper;
+
+ protected $controller_helper;
+
public function provider()
{
+ $this->controller_helper = $this->get_controller_helper();
// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
return array(
array('data://x', false, false, 'http://localhost/phpBB'),
@@ -26,13 +31,15 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
array('http://localhost/phpBB/app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
- array('./../app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
- array('./../app.php/foobar', true, false, 'http://../../../foobar'),
- array('./../app.php/foo/bar', false, false, 'http://localhost/phpBB/app.php/foo/bar'),
- array('./../app.php/foo/bar', true, false, 'http://../../../bar'),
- array('./../foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
- array('./../foo/bar', true, false, 'http://../../../bar'),
+ array('./../app.php/foobar', false, false, 'http://localhost/app.php/foobar'),
+ array('./../app.php/foobar', true, false, 'http://localhost/app.php/foobar'),
+ array('./../app.php/foo/bar', false, false, 'http://localhost/app.php/foo/bar'),
+ array('./../app.php/foo/bar', true, false, 'http://localhost/app.php/foo/bar'),
+ array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
+ array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
+ array($this->controller_helper->url('a'), false, false, 'http://localhost/phpBB/app.php/a'),
+ array($this->controller_helper->url(''), false, false, 'http://localhost/phpBB/app.php/'),
array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
array('foobar', false, false, 'http://localhost/phpBB/foobar'),
array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
@@ -46,6 +53,47 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
);
}
+ protected function get_path_helper()
+ {
+ if (!($this->path_helper instanceof \phpbb\path_helper))
+ {
+ $this->path_helper = new \phpbb\path_helper(
+ new \phpbb\symfony_request(
+ new phpbb_mock_request()
+ ),
+ new \phpbb\filesystem(),
+ $this->phpbb_root_path,
+ 'php'
+ );
+ }
+ return $this->path_helper;
+ }
+
+ protected function get_controller_helper()
+ {
+ if (!($this->controller_helper instanceof \phpbb\controller\helper))
+ {
+ global $phpbb_dispatcher;
+
+ $phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+ $this->user = $this->getMock('\phpbb\user');
+ $phpbb_path_helper = new \phpbb\path_helper(
+ new \phpbb\symfony_request(
+ new phpbb_mock_request()
+ ),
+ new \phpbb\filesystem(),
+ $phpbb_root_path,
+ $phpEx
+ );
+ $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context());
+
+ // We don't use mod_rewrite in these tests
+ $config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
+ $this->controller_helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php');
+ }
+ return $this->controller_helper;
+ }
+
protected function setUp()
{
parent::setUp();
@@ -54,14 +102,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
'force_server_vars' => '0',
);
- $this->path_helper = new \phpbb\path_helper(
- new \phpbb\symfony_request(
- new phpbb_mock_request()
- ),
- new \phpbb\filesystem(),
- $this->phpbb_root_path,
- 'php'
- );
+ $this->path_helper = $this->get_path_helper();
+ $this->controller_helper = $this->get_controller_helper();
}
/**
generated by cgit v1.2.1 (git 2.21.0) at 2024-11-11 06:28:27 +0000