From 60dda5577dfcbb3c5aac0fdd2a052e12293061a5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 8 Nov 2013 23:12:42 +0100 Subject: [ticket/11997] Correctly redirect to front controllers We currently do a lot of checks in order to prevent users from getting to a 404 page. However, this logic relies on checking if a file or folder exists. Due to the front controllers and the URL rewriting in 3.1, it is no longer possible to rely on existing files for redirecting. This patch will take care of properly redirecting users to front controller files. An incorrect link will cause users to get a 404 error though. PHPBB3-11997 --- phpBB/includes/functions.php | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 35fa785616..744e610f32 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -999,7 +999,9 @@ function phpbb_own_realpath($path) // @internal The slash in is_dir() gets around an open_basedir restriction if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved))) { - return false; + // In order to allow proper URL rewriting we need to allow + // paths that are non-existent + //return false; } // Put the slashes back to the native operating systems slashes @@ -2696,20 +2698,6 @@ function redirect($url, $return = false, $disable_cd_check = false) // Relative uri $pathinfo = pathinfo($url); - if (!$disable_cd_check && !file_exists($pathinfo['dirname'] . '/')) - { - $url = str_replace('../', '', $url); - $pathinfo = pathinfo($url); - - if (!file_exists($pathinfo['dirname'] . '/')) - { - // fallback to "last known user page" - // at least this way we know the user does not leave the phpBB root - $url = generate_board_url() . '/' . $user->page['page']; - $failover_flag = true; - } - } - if (!$failover_flag) { // Is the uri pointing to the current directory? @@ -2723,14 +2711,7 @@ function redirect($url, $return = false, $disable_cd_check = false) $url = substr($url, 1); } - if ($user->page['page_dir']) - { - $url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url; - } - else - { - $url = generate_board_url() . '/' . $url; - } + $url = generate_board_url() . '/' . $url; } else { @@ -2741,8 +2722,9 @@ function redirect($url, $return = false, $disable_cd_check = false) $root_dirs = array_diff_assoc($root_dirs, $intersection); $page_dirs = array_diff_assoc($page_dirs, $intersection); + $root_dirs_size = sizeof($root_dirs) - 2; - $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs); + $dir = (($root_dirs_size > 0) ? str_repeat('../', $root_dirs_size) : '') . implode('/', $page_dirs); // Strip / from the end if ($dir && substr($dir, -1, 1) == '/') -- cgit v1.2.1 From 6bb9bce40e0831e449d1843901a485eef66206d6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 10 Nov 2013 12:56:09 +0100 Subject: [ticket/11997] Remove obsolete if statement in phpbb_own_realpath() PHPBB3-11997 --- phpBB/includes/functions.php | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 744e610f32..229e856d05 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -994,16 +994,6 @@ function phpbb_own_realpath($path) $resolved .= $bit . (($i == $max) ? '' : '/'); } - // @todo If the file exists fine and open_basedir only has one path we should be able to prepend it - // because we must be inside that basedir, the question is where... - // @internal The slash in is_dir() gets around an open_basedir restriction - if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved))) - { - // In order to allow proper URL rewriting we need to allow - // paths that are non-existent - //return false; - } - // Put the slashes back to the native operating systems slashes $resolved = str_replace('/', DIRECTORY_SEPARATOR, $resolved); -- cgit v1.2.1 From 2d0fb4d166d0f6371b6c9c6a9e1dce2b34992c9e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 11 Nov 2013 21:55:21 +0100 Subject: [ticket/11997] Rename root_dirs_size to superordinate_dirs_count This variable name better fits what is done with it. A comment explaining why 2 has to be subtracted from the size of root_dirs has also been added PHPBB3-11997 --- phpBB/includes/functions.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 229e856d05..da90dfea10 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2712,9 +2712,17 @@ function redirect($url, $return = false, $disable_cd_check = false) $root_dirs = array_diff_assoc($root_dirs, $intersection); $page_dirs = array_diff_assoc($page_dirs, $intersection); - $root_dirs_size = sizeof($root_dirs) - 2; - $dir = (($root_dirs_size > 0) ? str_repeat('../', $root_dirs_size) : '') . implode('/', $page_dirs); + // Due to the dirname returned by pathinfo, the + // 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 if ($dir && substr($dir, -1, 1) == '/') -- cgit v1.2.1 From d43542a434c6a214c7533f76f3b1dc7afe84e5cf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 12 Nov 2013 00:46:43 +0100 Subject: [ticket/11997] Use $phpbb_filesystem->clean_path() for proper redirect paths PHPBB3-11997 --- phpBB/includes/functions.php | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index da90dfea10..0a10a9604c 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2645,7 +2645,7 @@ function generate_board_url($without_script_path = 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; @@ -2713,16 +2713,7 @@ function redirect($url, $return = false, $disable_cd_check = false) $root_dirs = array_diff_assoc($root_dirs, $intersection); $page_dirs = array_diff_assoc($page_dirs, $intersection); - // Due to the dirname returned by pathinfo, the - // 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); + $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs); // Strip / from the end 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); } + $url = $phpbb_filesystem->clean_path($url); + if ($return) { return $url; -- cgit v1.2.1 From 0a7db81426e966f9c44fcc85338615e1bb3c6f34 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 12 Nov 2013 22:25:23 +0100 Subject: [ticket/11997] Fix redirects from inside controllers The redirect url currently uses the web root path. However as we prepend the full board url later, we need to remove the relative web root path and prepend the normal root path again. Otherwise redirects from inside routes will not work as intended. PHPBB3-11997 --- phpBB/includes/functions.php | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 0a10a9604c..c937b8f562 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2662,6 +2662,16 @@ function redirect($url, $return = false, $disable_cd_check = false) // Make sure no &'s are in, this will break the redirect $url = str_replace('&', '&', $url); + // The url currently uses the web root path. + // However as we prepend the full board url later, + // we need to remove the relative web root path and + // prepend the normal root path again. Otherwise redirects + // from inside routes will not work as intended. + if ($phpbb_path_helper instanceof \phpbb\path_helper) + { + $url = $phpbb_path_helper->remove_web_root_path($url); + } + // Determine which type of redirect we need to handle... $url_parts = @parse_url($url); -- cgit v1.2.1 From 0aed2816761601ae902159665fdf58932209f772 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 Nov 2013 11:21:47 +0100 Subject: [ticket/11997] Fix missing global PHPBB3-11997 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index c937b8f562..a6c03098c2 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2645,7 +2645,7 @@ function generate_board_url($without_script_path = false) */ function redirect($url, $return = false, $disable_cd_check = false) { - global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem; + global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper; $failover_flag = false; -- cgit v1.2.1 From 68db335468c0395d8500d1e4e54ed28aca875a30 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 17 Nov 2013 20:16:02 +0100 Subject: [ticket/11997] Remove obsolete failover_flag in function redirect() This flag is no longer needed as the failover is no longer needed. PHPBB3-11997 --- phpBB/includes/functions.php | 77 +++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 41 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index a6c03098c2..0663d0cf85 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2647,8 +2647,6 @@ function redirect($url, $return = false, $disable_cd_check = false) { global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper; - $failover_flag = false; - if (empty($user->lang)) { $user->add_lang('common'); @@ -2698,56 +2696,53 @@ function redirect($url, $return = false, $disable_cd_check = false) // Relative uri $pathinfo = pathinfo($url); - if (!$failover_flag) + // Is the uri pointing to the current directory? + if ($pathinfo['dirname'] == '.') { - // 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); - } + $url = str_replace('./', '', $url); - $url = generate_board_url() . '/' . $url; - } - else + // Strip / from the beginning + if ($url && substr($url, 0, 1) == '/') { - // 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); + $url = substr($url, 1); + } - $root_dirs = array_diff_assoc($root_dirs, $intersection); - $page_dirs = array_diff_assoc($page_dirs, $intersection); + $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); - $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs); + $root_dirs = array_diff_assoc($root_dirs, $intersection); + $page_dirs = array_diff_assoc($page_dirs, $intersection); - // Strip / from the end - if ($dir && substr($dir, -1, 1) == '/') - { - $dir = substr($dir, 0, -1); - } + $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs); - // Strip / from the beginning - if ($dir && substr($dir, 0, 1) == '/') - { - $dir = substr($dir, 1); - } + // Strip / from the end + if ($dir && substr($dir, -1, 1) == '/') + { + $dir = substr($dir, 0, -1); + } - $url = str_replace($pathinfo['dirname'] . '/', '', $url); + // Strip / from the beginning + if ($dir && substr($dir, 0, 1) == '/') + { + $dir = substr($dir, 1); + } - // Strip / from the beginning - if (substr($url, 0, 1) == '/') - { - $url = substr($url, 1); - } + $url = str_replace($pathinfo['dirname'] . '/', '', $url); - $url = (!empty($dir) ? $dir . '/' : '') . $url; - $url = generate_board_url() . '/' . $url; + // Strip / from the beginning + if (substr($url, 0, 1) == '/') + { + $url = substr($url, 1); } + + $url = (!empty($dir) ? $dir . '/' : '') . $url; + $url = generate_board_url() . '/' . $url; } } -- cgit v1.2.1 From 8370857f0408b9610ba80e9bc06cde19c8e58983 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 7 Dec 2013 13:20:40 +0100 Subject: [ticket/11997] Undo changes to phpbb_own_realpath() PHPBB3-11997 --- phpBB/includes/functions.php | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 0663d0cf85..588a060630 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -994,6 +994,14 @@ function phpbb_own_realpath($path) $resolved .= $bit . (($i == $max) ? '' : '/'); } + // @todo If the file exists fine and open_basedir only has one path we should be able to prepend it + // because we must be inside that basedir, the question is where... + // @internal The slash in is_dir() gets around an open_basedir restriction + if (!@file_exists($resolved) || (!@is_dir($resolved . '/') && !is_file($resolved))) + { + return false; + } + // Put the slashes back to the native operating systems slashes $resolved = str_replace('/', DIRECTORY_SEPARATOR, $resolved); -- cgit v1.2.1 From a7f2788c72dd45b65de494ca72d13aaee3b140d6 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 7 Dec 2013 13:25:04 +0100 Subject: [ticket/11997] Use get_controller_redirect_url() in redirect() function This method of path_helper will now be used instead of the previous hack of the phpbb_own_realpath() function. PHPBB3-11997 --- phpBB/includes/functions.php | 116 ++++++++++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 46 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 588a060630..9569a6de82 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2655,6 +2655,8 @@ function redirect($url, $return = false, $disable_cd_check = false) { global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper; + $failover_flag = false; + if (empty($user->lang)) { $user->add_lang('common'); @@ -2668,16 +2670,6 @@ function redirect($url, $return = false, $disable_cd_check = false) // Make sure no &'s are in, this will break the redirect $url = str_replace('&', '&', $url); - // The url currently uses the web root path. - // However as we prepend the full board url later, - // we need to remove the relative web root path and - // prepend the normal root path again. Otherwise redirects - // from inside routes will not work as intended. - if ($phpbb_path_helper instanceof \phpbb\path_helper) - { - $url = $phpbb_path_helper->remove_web_root_path($url); - } - // Determine which type of redirect we need to handle... $url_parts = @parse_url($url); @@ -2704,53 +2696,87 @@ function redirect($url, $return = false, $disable_cd_check = false) // Relative uri $pathinfo = pathinfo($url); - // Is the uri pointing to the current directory? - if ($pathinfo['dirname'] == '.') + // Also treat URLs that have a non-existing basename + if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename']))) { - $url = str_replace('./', '', $url); + $url = str_replace('../', '', $url); + $pathinfo = pathinfo($url); - // Strip / from the beginning - if ($url && substr($url, 0, 1) == '/') + // Also treat URLs that have a non-existing basename + if (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename'])) { - $url = substr($url, 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 = 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); + if (!$failover_flag) + { + // Is the uri pointing to the current directory? + if ($pathinfo['dirname'] == '.') + { + $url = str_replace('./', '', $url); - $dir = str_repeat('../', sizeof($root_dirs)) . implode('/', $page_dirs); + // Strip / from the beginning + if ($url && substr($url, 0, 1) == '/') + { + $url = substr($url, 1); + } - // Strip / from the end - if ($dir && substr($dir, -1, 1) == '/') - { - $dir = substr($dir, 0, -1); + if ($user->page['page_dir']) + { + $url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $url; + } + else + { + $url = generate_board_url() . '/' . $url; + } } - - // Strip / from the beginning - if ($dir && substr($dir, 0, 1) == '/') + else { - $dir = substr($dir, 1); - } + // 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); - $url = str_replace($pathinfo['dirname'] . '/', '', $url); + $root_dirs = array_diff_assoc($root_dirs, $intersection); + $page_dirs = array_diff_assoc($page_dirs, $intersection); - // Strip / from the beginning - if (substr($url, 0, 1) == '/') - { - $url = substr($url, 1); - } + $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 = (!empty($dir) ? $dir . '/' : '') . $url; - $url = generate_board_url() . '/' . $url; + // Strip / from the beginning + if (substr($url, 0, 1) == '/') + { + $url = substr($url, 1); + } + + $url = (!empty($dir) ? $dir . '/' : '') . $url; + $url = generate_board_url() . '/' . $url; + } + $url = $phpbb_filesystem->clean_path($url); } } @@ -2769,8 +2795,6 @@ function redirect($url, $return = false, $disable_cd_check = false) trigger_error('INSECURE_REDIRECT', E_USER_ERROR); } - $url = $phpbb_filesystem->clean_path($url); - if ($return) { return $url; -- cgit v1.2.1 From 235d2069e0e7cecfd51d4eed5c875cc865f35486 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 21 Dec 2013 16:31:20 +0100 Subject: [ticket/11997] Allow redirects to parent folders like previously Redirects to parent folders were possible with the previous redirect function. This change will allow these redirects again. PHPBB3-11997 --- phpBB/includes/functions.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index d874b7b19e..4c9c3323f7 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2653,7 +2653,7 @@ function generate_board_url($without_script_path = false) */ function redirect($url, $return = false, $disable_cd_check = false) { - global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper; + global $db, $cache, $config, $user, $phpbb_root_path, $phpbb_filesystem, $phpbb_path_helper, $phpEx; $failover_flag = false; @@ -2696,14 +2696,15 @@ function redirect($url, $return = false, $disable_cd_check = false) // Relative uri $pathinfo = pathinfo($url); - // Also treat URLs that have a non-existing basename - if (!$disable_cd_check && (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename']))) + // 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)))) { $url = str_replace('../', '', $url); $pathinfo = pathinfo($url); // Also treat URLs that have a non-existing basename - if (!file_exists($pathinfo['dirname'] . '/') || !file_exists($pathinfo['basename'])) + if (!file_exists($pathinfo['dirname'] . '/') || (!file_exists($url) && preg_match('/^[\.]?+[\/]?+(?:app\.php)?+[a-zA-Z0-9\/]/', $url))) { // fallback to "last known user page" // at least this way we know the user does not leave the phpBB root -- cgit v1.2.1 From d9358c26da6737044a3c10893e7b954176b205d2 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 21 Dec 2013 20:08:00 +0100 Subject: [ticket/11997] Add clean_url() method to path_helper This method will get rid of unnecessary . and .. in URLs. PHPBB3-11997 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 4c9c3323f7..aea13f7679 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2777,7 +2777,7 @@ function redirect($url, $return = false, $disable_cd_check = false) $url = (!empty($dir) ? $dir . '/' : '') . $url; $url = generate_board_url() . '/' . $url; } - $url = $phpbb_filesystem->clean_path($url); + $url = $phpbb_path_helper->clean_url($url);; } } -- cgit v1.2.1 From 9161816267ace60d9972e01b7fbe1213fe29a708 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 27 Dec 2013 13:00:22 +0100 Subject: [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 --- phpBB/includes/functions.php | 88 +++++++------------------------------------- 1 file changed, 13 insertions(+), 75 deletions(-) (limited to 'phpBB/includes/functions.php') 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 -- cgit v1.2.1 From a304d99b2b5d0b69548b81b013aa711838ff521d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 27 Dec 2013 17:50:17 +0100 Subject: [ticket/11997] Add remove_web_root_path() in order to prevent incorrect URLs Adding a call to this method to the redirect function will make sure that we do not end up with incorrect URLs after using append_sid(). PHPBB3-11997 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 33d0e36a28..bd6a45685d 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2708,7 +2708,7 @@ function redirect($url, $return = false, $disable_cd_check = false) } } - $url = generate_board_url() . '/' . $url; + $url = generate_board_url() . '/' . $phpbb_path_helper->remove_web_root_path($url); } // Clean URL and check if we go outside the forum directory -- cgit v1.2.1 From 4c1569dd8ab6d85aecd0bd30913512542d1f123d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 28 Dec 2013 00:14:12 +0100 Subject: [ticket/11997] Add user's page dir to redirect path and fix unit tests for it The user's page directory needs to be added to the redirect URL for proper redirects outside of the forum root. Fix the unit tests accordingly. PHPBB3-11997 --- phpBB/includes/functions.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'phpBB/includes/functions.php') diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index bd6a45685d..cc8478ace1 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2708,7 +2708,14 @@ function redirect($url, $return = false, $disable_cd_check = false) } } - $url = generate_board_url() . '/' . $phpbb_path_helper->remove_web_root_path($url); + $url = $phpbb_path_helper->remove_web_root_path($url); + + if ($user->page['page_dir']) + { + $url = $user->page['page_dir'] . '/' . $url; + } + + $url = generate_board_url() . '/' . $url; } // Clean URL and check if we go outside the forum directory -- cgit v1.2.1