aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTristan Darricau <github@nicofuma.fr>2014-11-07 11:10:03 +0100
committerTristan Darricau <github@nicofuma.fr>2014-11-07 11:10:03 +0100
commita1b58d05d158ff7afd789c1b27821e17198f8d58 (patch)
tree41c5955e720b462173e78fed5c18c18057e0b62b
parent0e772afb9db640e54e84cfccaddcf74f3edbb3fb (diff)
parentf432193e0789f0182741d1b4a38a8c4cd86dfba4 (diff)
downloadforums-a1b58d05d158ff7afd789c1b27821e17198f8d58.tar
forums-a1b58d05d158ff7afd789c1b27821e17198f8d58.tar.gz
forums-a1b58d05d158ff7afd789c1b27821e17198f8d58.tar.bz2
forums-a1b58d05d158ff7afd789c1b27821e17198f8d58.tar.xz
forums-a1b58d05d158ff7afd789c1b27821e17198f8d58.zip
Merge pull request #3107 from marc1706/ticket/13280
[ticket/13280] Properly format the current page and add sanitizer to tests * marc1706/ticket/13280: [ticket/13280] Remove unneeded str_replace in build_url() [ticket/13280] Only run sanitizer for server superglobal and modify tests [ticket/13280] Seperate server sanitizer call and add comment [ticket/13280] Add additional sanitizer for ampersands in server superglobal [ticket/13280] Correctly format user page for build_url() [ticket/13280] Properly format the current page and add sanitizer to tests
Diffstat
-rw-r--r--phpBB/phpbb/session.php6
-rw-r--r--phpBB/phpbb/symfony_request.php10
-rw-r--r--tests/functions/build_url_test.php6
-rw-r--r--tests/security/extract_current_page_test.php28
4 files changed, 38 insertions, 12 deletions
diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php
index 14b4c63207..a06ff9c594 100644
--- a/phpBB/phpbb/session.php
+++ b/phpBB/phpbb/session.php
@@ -43,7 +43,7 @@ class session
// First of all, get the request uri...
$script_name = $symfony_request->getScriptName();
- $args = explode('&', $symfony_request->getQueryString());
+ $args = explode('&amp;', $symfony_request->getQueryString());
// 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)
@@ -61,8 +61,8 @@ class session
// Since some browser do not encode correctly we need to do this with some "special" characters...
// " -> %22, ' => %27, < -> %3C, > -> %3E
- $find = array('"', "'", '<', '>');
- $replace = array('%22', '%27', '%3C', '%3E');
+ $find = array('"', "'", '<', '>', '&quot;', '&lt;', '&gt;');
+ $replace = array('%22', '%27', '%3C', '%3E', '%22', '%3C', '%3E');
foreach ($args as $key => $argument)
{
diff --git a/phpBB/phpbb/symfony_request.php b/phpBB/phpbb/symfony_request.php
index ad949a35f2..02d22c480f 100644
--- a/phpBB/phpbb/symfony_request.php
+++ b/phpBB/phpbb/symfony_request.php
@@ -30,6 +30,12 @@ class symfony_request extends Request
$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('&amp;', '&', $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);
@@ -38,10 +44,12 @@ class symfony_request extends Request
array_walk_recursive($get_parameters, $sanitizer);
array_walk_recursive($post_parameters, $sanitizer);
- array_walk_recursive($server_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/functions/build_url_test.php b/tests/functions/build_url_test.php
index 06415a424e..5cfd1300de 100644
--- a/tests/functions/build_url_test.php
+++ b/tests/functions/build_url_test.php
@@ -69,6 +69,11 @@ class phpbb_build_url_test extends phpbb_test_case
array('f', 'style', 't'),
'http://test.phpbb.com/viewtopic.php?',
),
+ array(
+ 'posting.php?f=2&mode=delete&p=20%22%3Cscript%3Ealert%281%29%3B%3C%2Fscript%3E',
+ false,
+ 'phpBB/posting.php?f=2&amp;mode=delete&amp;p=20%22%3Cscript%3Ealert%281%29%3B%3C%2Fscript%3E',
+ )
);
}
@@ -80,6 +85,7 @@ 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/security/extract_current_page_test.php b/tests/security/extract_current_page_test.php
index 58dea68dc8..c127b69b2b 100644
--- a/tests/security/extract_current_page_test.php
+++ b/tests/security/extract_current_page_test.php
@@ -37,16 +37,16 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base
));
$symfony_request->expects($this->any())
->method('getScriptName')
- ->will($this->returnValue($url));
+ ->will($this->returnValue($this->sanitizer($url)));
$symfony_request->expects($this->any())
->method('getQueryString')
- ->will($this->returnValue($query_string));
+ ->will($this->returnValue($this->sanitizer($query_string)));
$symfony_request->expects($this->any())
->method('getBasePath')
->will($this->returnValue($server['REQUEST_URI']));
- $symfony_request->expects($this->any())
+ $symfony_request->expects($this->sanitizer($this->any()))
->method('getPathInfo')
- ->will($this->returnValue('/'));
+ ->will($this->returnValue($this->sanitizer('/')));
$result = \phpbb\session::extract_current_page('./');
$label = 'Running extract_current_page on ' . $query_string . ' with PHP_SELF filled.';
@@ -65,20 +65,32 @@ class phpbb_security_extract_current_page_test extends phpbb_security_test_base
));
$symfony_request->expects($this->any())
->method('getScriptName')
- ->will($this->returnValue($url));
+ ->will($this->returnValue($this->sanitizer($url)));
$symfony_request->expects($this->any())
->method('getQueryString')
- ->will($this->returnValue($query_string));
+ ->will($this->returnValue($this->sanitizer($query_string)));
$symfony_request->expects($this->any())
->method('getBasePath')
- ->will($this->returnValue($server['REQUEST_URI']));
+ ->will($this->returnValue($this->sanitizer($server['REQUEST_URI'])));
$symfony_request->expects($this->any())
->method('getPathInfo')
- ->will($this->returnValue('/'));
+ ->will($this->returnValue($this->sanitizer('/')));
$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('&amp;', '&', $value);
+ }
}
generated by cgit v1.2.1 (git 2.21.0) at 2025-03-29 06:07:00 +0000