From d07e152ea7e820c5a0e47aeb8004fa0b5621a314 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sat, 13 Mar 2010 01:54:04 +0000 Subject: [PATCH 1/4] [bug/58025] Search robots are now redirected if they send a SID in the request Previously search robots could stumble upon a board link somewhere on the web containing a SID they'd follow it and end up indexing that page with the SID in the request URI, this fix prevents that by redirecting them to the same URI just without the SID. --- phpBB/docs/CHANGELOG.html | 1 + phpBB/includes/session.php | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/phpBB/docs/CHANGELOG.html b/phpBB/docs/CHANGELOG.html index 7b8d8f63f2..7df49bd81b 100644 --- a/phpBB/docs/CHANGELOG.html +++ b/phpBB/docs/CHANGELOG.html @@ -103,6 +103,7 @@
  • [Fix] Allow multibyte keys in request_var(). (Bug #51555)
  • [Fix] Prevent wrong tar archive type detection. (Bug #12531)
  • [Fix] Correct redirection after login to forum not in web root (Bug #58755)
    • +
  • [Fix] Redirect search engines that access pages with SIDs in the URL. (Bug #58025)
  • [Feature] Support for Microsoft's Native SQL Server Driver for PHP (Bug #57055 - Patch by Chris Pucci at Microsoft)
    • diff --git a/phpBB/includes/session.php b/phpBB/includes/session.php index 1a302d5991..8beb0161f9 100644 --- a/phpBB/includes/session.php +++ b/phpBB/includes/session.php @@ -608,6 +608,12 @@ class session } else { + // Bot user, if they have a SID in the Request URI we need to get rid of it + // otherwise they'll index this page with the SID, duplicate content oh my! + if (isset($_GET['sid'])) + { + redirect(build_url(array('sid'))); + } $this->data['session_last_visit'] = $this->time_now; } From b64686073aaeb00feaf9d285c8cf62f3e677544b Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Sat, 6 Mar 2010 18:02:26 +0000 Subject: [PATCH 2/4] [bug/58685] Correct spelling errors in append_sid() comments. --- phpBB/includes/functions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 917433a970..ce1f5f5462 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2139,8 +2139,8 @@ function append_sid($url, $params = false, $is_amp = true, $session_id = false) { global $_SID, $_EXTRA_URL, $phpbb_hook; - // Developers using the hook function need to globalise the $_SID and $_EXTRA_URL on their own and also handle it appropiatly. - // They could mimick most of what is within this function + // Developers using the hook function need to globalise the $_SID and $_EXTRA_URL on their own and also handle it appropriately. + // They could mimic most of what is within this function if (!empty($phpbb_hook) && $phpbb_hook->call_hook(__FUNCTION__, $url, $params, $is_amp, $session_id)) { if ($phpbb_hook->hook_return(__FUNCTION__)) From eca2db4afefdfbb256d61fae3f942835848f04e0 Mon Sep 17 00:00:00 2001 From: Josh Woody Date: Wed, 17 Mar 2010 23:24:34 -0500 Subject: [PATCH 3/4] [bug/56965] Redirect fails with directory traversal Correct invalid r10536 with a boolean flag. Note that this fix for the bug will not actually correct the redirects, it will only prevent phpBB from redirecting outside $phpbb_root_path when redirect()'s third argument is not provided. --- phpBB/includes/functions.php | 100 +++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 38f910974a..be949a1258 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2297,6 +2297,8 @@ function redirect($url, $return = false, $disable_cd_check = false) { global $db, $cache, $config, $user, $phpbb_root_path; + $failover_flag = false; + if (empty($user->lang)) { $user->add_lang('common'); @@ -2344,66 +2346,70 @@ function redirect($url, $return = false, $disable_cd_check = false) 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']; - break; + $failover_flag = true; } } - // Is the uri pointing to the current directory? - if ($pathinfo['dirname'] == '.') + if (!$failover_flag) { - $url = str_replace('./', '', $url); - - // Strip / from the beginning - if ($url && substr($url, 0, 1) == '/') + // Is the uri pointing to the current directory? + if ($pathinfo['dirname'] == '.') { - $url = substr($url, 1); - } + $url = str_replace('./', '', $url); - if ($user->page['page_dir']) - { - $url = generate_board_url() . '/' . $user->page['page_dir'] . '/' . $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); + + // Strip / from the beginning + if (substr($url, 0, 1) == '/') + { + $url = substr($url, 1); + } + + $url = (!empty($dir) ? $dir . '/' : '') . $url; $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); - - // Strip / from the beginning - if (substr($url, 0, 1) == '/') - { - $url = substr($url, 1); - } - - $url = (!empty($dir) ? $dir . '/' : '') . $url; - $url = generate_board_url() . '/' . $url; - } } // Make sure no linebreaks are there... to prevent http response splitting for PHP < 4.4.2 From c71b1245ecf60d7b8b1143b1f558fcd2a974383a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 26 Mar 2010 14:46:18 +0100 Subject: [PATCH 4/4] [develop-olympus] Make this test run on windows with backslash-paths. --- tests/template/template.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/template/template.php b/tests/template/template.php index df12f92046..145fe8de61 100644 --- a/tests/template/template.php +++ b/tests/template/template.php @@ -264,7 +264,7 @@ class phpbb_template_template_test extends phpbb_test_case $this->template->set_filenames(array('test' => $filename)); $this->assertFileNotExists($this->template_path . '/' . $filename, 'Testing missing file, file cannot exist'); - $expecting = sprintf('template->_tpl_load_file(): File %s does not exist or is empty', realpath($this->template_path) . '/' . $filename); + $expecting = sprintf('template->_tpl_load_file(): File %s does not exist or is empty', realpath($this->template_path . '/../') . '/templates/' . $filename); $this->setExpectedTriggerError(E_USER_ERROR, $expecting); $this->display('test');