makary/phpbb
1
0
Fork 0
mirror of https://github.com/phpbb/phpbb.git synced 2025-06-08 04:18:52 +00:00
Code Issues Projects Releases Packages Wiki Activity

[ticket/11997] Use $phpbb_filesystem->clean_path() for proper redirect paths

Browse source 
PHPBB3-11997
This commit is contained in:
Marc Alexander 2013-11-12 00:46:43 +01:00
parent 2d0fb4d166
commit d43542a434
2 changed files with 39 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
phpBB/includes/functions.php
View file

@ -2645,7 +2645,7 @@ function generate_board_url($without_script_path = false)
*/ */
function redirect($url, $return = false, $disable_cd_check = false) function redirect($url, $return = false, $disable_cd_check = false)
{ {
global $db, $cache, $config, $user, $phpbb_root_path; global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem;
$failover_flag = false; $failover_flag = false;
@ -2713,16 +2713,7 @@ function redirect($url, $return = false, $disable_cd_check = false)
$root_dirs = array_diff_assoc($root_dirs, $intersection); $root_dirs = array_diff_assoc($root_dirs, $intersection);
$page_dirs = array_diff_assoc($page_dirs, $intersection); $page_dirs = array_diff_assoc($page_dirs, $intersection);
// Due to the dirname returned by pathinfo, the $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs);
// realpath for the $page_dirs array will be 2
// superordinate folders up from the phpBB root
// path even if the supplied path is in the
// phpBB root path. We need to subtract these 2
// folders in order to be able to correctly
// prepend '../' to the supplied path.
$superordinate_dirs_count = sizeof($root_dirs) - 2;
$dir = (($superordinate_dirs_count > 0) ? str_repeat('../', $superordinate_dirs_count) : '') . implode('/', $page_dirs);
// Strip / from the end // Strip / from the end
if ($dir && substr($dir, -1, 1) == '/') if ($dir && substr($dir, -1, 1) == '/')
@ -2765,6 +2756,8 @@ function redirect($url, $return = false, $disable_cd_check = false)
trigger_error('INSECURE_REDIRECT', E_USER_ERROR); trigger_error('INSECURE_REDIRECT', E_USER_ERROR);
} }
$url = $phpbb_filesystem->clean_path($url);
if ($return) if ($return)
{ {
return $url; return $url;

58
tests/security/redirect_test.php
View file

@ -17,26 +17,32 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
{ {
// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false)) // array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
return array( return array(
array('data://x', false, 'http://localhost/phpBB'), array('data://x', false, false, 'http://localhost/phpBB'),
array('bad://localhost/phpBB/index.php', 'INSECURE_REDIRECT', false), array('bad://localhost/phpBB/index.php', false, 'INSECURE_REDIRECT', false),
array('http://www.otherdomain.com/somescript.php', false, 'http://localhost/phpBB'), array('http://www.otherdomain.com/somescript.php', false, false, 'http://localhost/phpBB'),
array("http://localhost/phpBB/memberlist.php\n\rConnection: close", 'INSECURE_REDIRECT', false), array("http://localhost/phpBB/memberlist.php\n\rConnection: close", false, 'INSECURE_REDIRECT', false),
array('javascript:test', false, 'http://localhost/phpBB/javascript:test'), array('javascript:test', false, false, 'http://localhost/phpBB/javascript:test'),
array('http://localhost/phpBB/index.php;url=', 'INSECURE_REDIRECT', false), array('http://localhost/phpBB/index.php;url=', false, 'INSECURE_REDIRECT', false),
array('http://localhost/phpBB/app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'), array('http://localhost/phpBB/app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
array('./app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'), array('./app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
array('app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'), array('app.php/foobar', false, false, 'http://localhost/phpBB/app.php/foobar'),
array('./../app.php/foobar', false, 'http://localhost/phpBB/app.php/foobar'), array('./../app.php/foobar', false, false, 'http://localhost/app.php/foobar'),
array('./../app.php/foo/bar', false, 'http://localhost/phpBB/app.php/foo/bar'), array('./../app.php/foobar', true, false, 'http://localhost/app.php/foobar'),
array('./../foo/bar', false, 'http://localhost/phpBB/foo/bar'), array('./../app.php/foo/bar', false, false, 'http://localhost/app.php/foo/bar'),
array('app.php/', false, 'http://localhost/phpBB/app.php/'), array('./../app.php/foo/bar', true, false, 'http://localhost/app.php/foo/bar'),
array('./app.php/', false, 'http://localhost/phpBB/app.php/'), array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
array('foobar', false, 'http://localhost/phpBB/foobar'), array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
array('./foobar', false, 'http://localhost/phpBB/foobar'), array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
array('foo/bar', false, 'http://localhost/phpBB/foo/bar'), array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
array('./foo/bar', false, 'http://localhost/phpBB/foo/bar'), array('foobar', false, false, 'http://localhost/phpBB/foobar'),
array('./../index.php', false, 'http://localhost/phpBB/index.php'), array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
array('../index.php', false, 'http://localhost/phpBB/index.php'), array('foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
array('./foo/bar', false, false, 'http://localhost/phpBB/foo/bar'),
array('./../index.php', false, false, 'http://localhost/index.php'),
array('./../index.php', true, false, 'http://localhost/index.php'),
array('../index.php', false, false, 'http://localhost/index.php'),
array('../index.php', true, false, 'http://localhost/index.php'),
array('./index.php', false, false, 'http://localhost/phpBB/index.php'),
); );
} }
@ -52,21 +58,27 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
/** /**
* @dataProvider provider * @dataProvider provider
*/ */
public function test_redirect($test, $expected_error, $expected_result) public function test_redirect($test, $disable_cd_check, $expected_error, $expected_result)
{ {
global $user; global $user, $phpbb_root_path;
$temp_phpbb_root_path = $phpbb_root_path;
// We need to hack phpbb_root_path here, so it matches the actual fileinfo of the testing script.
// Otherwise the paths are returned incorrectly.
$phpbb_root_path = '';
if ($expected_error !== false) if ($expected_error !== false)
{ {
$this->setExpectedTriggerError(E_USER_ERROR, $expected_error); $this->setExpectedTriggerError(E_USER_ERROR, $expected_error);
} }
$result = redirect($test, true); $result = redirect($test, true, $disable_cd_check);
// only verify result if we did not expect an error // only verify result if we did not expect an error
if ($expected_error === false) if ($expected_error === false)
{ {
$this->assertEquals($expected_result, $result); $this->assertEquals($expected_result, $result);
} }
$phpbb_root_path = $temp_phpbb_root_path;
} }
} }