diff options
| -rw-r--r-- | core/API/Request.php | 7 | ||||
| -rw-r--r-- | core/Common.php | 4 | ||||
| -rw-r--r-- | core/FrontController.php | 1 | ||||
| -rw-r--r-- | core/Tracker.php | 2 | ||||
| -rw-r--r-- | core/Tracker/Generator.php | 4 | ||||
| -rw-r--r-- | core/ViewDataTable.php | 4 | ||||
| -rwxr-xr-x | tests/core/Common.test.php | 28 | ||||
| -rw-r--r-- | tests/resources/referer-xss.txt | 26 |
8 files changed, 44 insertions, 32 deletions
diff --git a/core/API/Request.php b/core/API/Request.php index 79993992ac..60bef14822 100644 --- a/core/API/Request.php +++ b/core/API/Request.php @@ -47,12 +47,13 @@ class Piwik_API_Request * * @param string GET request that defines the API call (must at least contain a "method" parameter) * Example: method=UserSettings.getWideScreen&idSite=1&date=yesterday&period=week&format=xml - * If a request is not provided, then we use the $_REQUEST superglobal and fetch + * If a request is not provided, then we use the $_GET and $_POST superglobal and fetch * the values directly from the HTTP GET query. */ function __construct($request = null) { - $requestArray = $_REQUEST; + $defaultRequest = $_GET + $_POST; + $requestArray = $defaultRequest; if(!is_null($request)) { @@ -67,7 +68,7 @@ class Piwik_API_Request Zend_Registry::get('access')->reloadAccess(); } - $requestArray = array_merge($_REQUEST, $requestArray); + $requestArray = $requestArray + $defaultRequest; } foreach($requestArray as &$element) diff --git a/core/Common.php b/core/Common.php index 4d3b38815f..c3e167cffe 100644 --- a/core/Common.php +++ b/core/Common.php @@ -376,7 +376,7 @@ class Piwik_Common } /** - * Returns a sanitized variable value from the $_REQUEST superglobal. + * Returns a sanitized variable value from the $_GET and $_POST superglobal. * If the variable doesn't have a value or an empty value, returns the defaultValue if specified. * If the variable doesn't have neither a value nor a default value provided, an exception is raised. * @@ -395,7 +395,7 @@ class Piwik_Common { if(is_null($requestArrayToUse)) { - $requestArrayToUse = $_REQUEST; + $requestArrayToUse = $_GET + $_POST; } $varDefault = self::sanitizeInputValues( $varDefault ); diff --git a/core/FrontController.php b/core/FrontController.php index 1c2948a0a1..aa2fc38a59 100644 --- a/core/FrontController.php +++ b/core/FrontController.php @@ -280,7 +280,6 @@ class Piwik_FrontController for ($i=1; $i < $_SERVER['argc']; $i++) { parse_str($_SERVER['argv'][$i],$tmp); - $_REQUEST = array_merge($_REQUEST, $tmp); $_GET = array_merge($_GET, $tmp); } } diff --git a/core/Tracker.php b/core/Tracker.php index bad522f78b..2dfaa9ef3c 100644 --- a/core/Tracker.php +++ b/core/Tracker.php @@ -47,7 +47,7 @@ class Piwik_Tracker public function __construct() { - $this->request = $_REQUEST; + $this->request = $_GET + $_POST; } public function main() diff --git a/core/Tracker/Generator.php b/core/Tracker/Generator.php index ea1178123f..c52b1d904f 100644 --- a/core/Tracker/Generator.php +++ b/core/Tracker/Generator.php @@ -112,7 +112,7 @@ class Piwik_Tracker_Generator */ public function __construct() { - $_COOKIE = $_GET = $_REQUEST = $_POST = array(); + $_COOKIE = $_GET = $_POST = array(); // init GET and REQUEST to the empty array $this->setFakeRequest(); @@ -582,7 +582,7 @@ class Piwik_Tracker_Generator */ protected function setFakeRequest() { - $_REQUEST = $_GET = $this->currentget; + $_GET = $this->currentget; } /** diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php index bf4340c30d..c450cc80f5 100644 --- a/core/ViewDataTable.php +++ b/core/ViewDataTable.php @@ -566,9 +566,9 @@ abstract class Piwik_ViewDataTable */ protected function getDefaultOrCurrent( $nameVar ) { - if(isset($_REQUEST[$nameVar])) + if(isset($_GET[$nameVar])) { - return htmlspecialchars($_REQUEST[$nameVar]); + return htmlspecialchars($_GET[$nameVar]); } $default = $this->getDefault($nameVar); return $default; diff --git a/tests/core/Common.test.php b/tests/core/Common.test.php index 02d17129f2..3b5bd88dff 100755 --- a/tests/core/Common.test.php +++ b/tests/core/Common.test.php @@ -17,7 +17,7 @@ class Test_Piwik_Common extends UnitTestCase public function setUp() { - $_REQUEST = $_GET = $_POST = array(); + $_GET = $_POST = array(); } public function tearDown() @@ -194,7 +194,7 @@ class Test_Piwik_Common extends UnitTestCase */ function test_getRequestVar_emptyVarName() { - $_REQUEST['']=1; + $_GET['']=1; try { $test = Piwik_Common::getRequestVar(''); $this->fail("Exception not raised."); @@ -223,8 +223,8 @@ class Test_Piwik_Common extends UnitTestCase */ function test_getRequestVar_nodefaultNotypeWithValue() { - $_REQUEST['test'] = 1413.431413; - $this->assertEqual( Piwik_Common::getRequestVar('test'), $_REQUEST['test']); + $_GET['test'] = 1413.431413; + $this->assertEqual( Piwik_Common::getRequestVar('test'), $_GET['test']); } @@ -233,11 +233,11 @@ class Test_Piwik_Common extends UnitTestCase */ function test_getRequestVar_nodefaultWithtypeWithValue() { - $_REQUEST['test'] = 1413.431413; + $_GET['test'] = 1413.431413; try { $this->assertEqual( Piwik_Common::getRequestVar('test', null, 'string'), - (string)$_REQUEST['test']); + (string)$_GET['test']); $this->fail("Exception not raised."); } catch (Exception $expected) { @@ -268,7 +268,7 @@ class Test_Piwik_Common extends UnitTestCase function test_getRequestVar_withdefaultWithtypeWithValue() { - $_REQUEST['test'] = 1413.431413; + $_GET['test'] = 1413.431413; $this->assertEqual( Piwik_Common::getRequestVar('test', 2, 'int'), 2); } @@ -298,9 +298,9 @@ class Test_Piwik_Common extends UnitTestCase */ function test_getRequestVar_integerdefault() { - $_REQUEST['test'] = 1413.431413; + $_GET['test'] = 1413.431413; $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'int'), 45); - $_REQUEST['test'] = ''; + $_GET['test'] = ''; $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'int'), 45); $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'integer'), 45); $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'numeric'), 45); @@ -314,10 +314,10 @@ class Test_Piwik_Common extends UnitTestCase */ function test_getRequestVar_stringdefault() { - $_REQUEST['test'] = "1413.431413"; + $_GET['test'] = "1413.431413"; $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'int'), 45); $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'string'), "1413.431413"); - $_REQUEST['test'] = ''; + $_GET['test'] = ''; $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'string'), '45'); $this->assertEqual( Piwik_Common::getRequestVar('test', "geaga", 'string'), "geaga"); $this->assertEqual( Piwik_Common::getRequestVar('test', "'}{}}{}{}'", 'string'), "'}{}}{}{}'"); @@ -332,14 +332,14 @@ class Test_Piwik_Common extends UnitTestCase function test_getRequestVar_arraydefault() { $test = array("test", 1345524, array("gaga")); - $_REQUEST['test'] = $test; + $_GET['test'] = $test; $this->assertEqual( Piwik_Common::getRequestVar('test', array(), 'array'), $test); $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'string'), "45"); $this->assertEqual( Piwik_Common::getRequestVar('test', array(1), 'array'), $test); $this->assertEqual( Piwik_Common::getRequestVar('test', 4, 'int'), 4); - $_REQUEST['test'] = ''; + $_GET['test'] = ''; $this->assertEqual( Piwik_Common::getRequestVar('test', array(1), 'array'), array(1)); $this->assertEqual( Piwik_Common::getRequestVar('test', array(), 'array'), array()); } @@ -352,7 +352,7 @@ class Test_Piwik_Common extends UnitTestCase function test_getRequestVar_stringedNumericCastedNumeric() { $test = "45645646"; - $_REQUEST['test'] = $test; + $_GET['test'] = $test; $this->assertEqual( Piwik_Common::getRequestVar('test', 1, 'int'), 45645646); $this->assertEqual( Piwik_Common::getRequestVar('test', 45, 'integer'), 45645646); diff --git a/tests/resources/referer-xss.txt b/tests/resources/referer-xss.txt index b396c04813..9bee9422d1 100644 --- a/tests/resources/referer-xss.txt +++ b/tests/resources/referer-xss.txt @@ -1,7 +1,19 @@ -INSERT INTO `piwik_log_visit` (`idvisit`, `idsite`, `visitor_localtime`, `visitor_idcookie`, `visitor_returning`, `visit_first_action_time`, `visit_last_action_time`, `visit_server_date`, `visit_exit_idaction`, `visit_entry_idaction`, `visit_total_actions`, `visit_total_time`, `visit_goal_converted`, `referer_type`, `referer_name`, `referer_url`, `referer_keyword`, `config_md5config`, `config_os`, `config_browser_name`, `config_browser_version`, `config_resolution`, `config_pdf`, `config_flash`, `config_java`, `config_director`, `config_quicktime`, `config_realplayer`, `config_windowsmedia`, `config_cookie`, `location_ip`, `location_browser_lang`, `location_country`, `location_continent`, `location_provider`) VALUES -(5, 1, '14:38:01', 'fb9af1315358d20049619db26b6f1ff9', 1, '2008-11-14 15:48:40', '2008-11-14 15:48:40', '2008-11-14', 2, 2, 1, 10, '', 2, 'Google', 'http://www.google.co.uk/search?hl=en&q=%3Cscript%3Ealert(%27test%27);%3C/script%3E', '%3cscript%3ealert(%27test%27);%3c/script%3e', '231ea91c00491cb5e6484f00c274b037', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 0, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'), -(15, 1, '18:03:29', 'fff3699b375db5e7cea33a58454cb8a0', 0, '2008-11-14 18:04:39', '2008-11-14 18:04:39', '2008-11-14', 2, 2, 1, 10, '', 3, 'htmlentities', 'http://example.com/"<script>alert(''test'');</script>', '', '00b29dee0697cb1eeb1931d04813f5f1', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 1, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'), -(16, 1, '19:51:00', 'fb9af1315358d20049619db26b6f1ff9', 1, '2008-11-14 19:51:00', '2008-11-14 19:51:00', '2008-11-14', 5, 5, 1, 10, '', 3, 'example1.com', 'http://example.com/%22%3E%3Cscript%3Ealert(%27yo%27)%3C%2Fscript%3E', '', '00b29dee0697cb1eeb1931d04813f5f1', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 1, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'), -(17, 1, '10:25:48', 'fb9af1315358d20049619db26b6f1ff9', 1, '2008-11-14 10:25:48', '2008-11-14 10:25:48', '2008-11-14', 5, 5, 1, 10, '', 3, 'urlencode', 'http://example3.com/test%3cscript%3ealert(%27test%27);%3c/script%3e', '', '00b29dee0697cb1eeb1931d04813f5f1', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 1, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'), -(21, 1, '12:35:41', 'fb9af1315358d20049619db26b6f1ff9', 1, '2008-11-14 12:35:41', '2008-11-14 13:27:14', '2008-11-14', 5, 2, 26, 3093, '', 3, 'example2.com', 'http://example.com/"><script>alert(''hi'')</script>', '', '00b29dee0697cb1eeb1931d04813f5f1', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 1, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'), -(53615, 1, '14:38:01', 'fb9af1315358d20049619db26b6f1ff9', 1, '2008-11-24 14:04:42', '2008-11-24 14:21:20', '2008-11-24', 2, 2, 3, 998, '', 3, 'localhost<script>alert(''test'')<', 'http://localhost<script>alert(''test'')</script>/test<script>alert(''test'')</script>', '', '231ea91c00491cb5e6484f00c274b037', 'WXP', 'FF', '3.0', '1440x900', 1, 1, 0, 0, 0, 0, 1, 1, 2130706433, 'en-gb,fr;q=0.8,ja;q=', 'uk', 'eur', 'Ip'); +Manual regression test procedure for XSS referer +================================================ +1. set in the config.ini.php +[Tracker] +visit_standard_length = 1 +enable_detect_unique_visitor_using_settings = false +[Debug] +always_archive_data = true + +2. go to /misc/testJavascriptTracker/ and fake the referer using, eg. RefControl options Firefox extension +http://www.google.co.uk/search?hl=en&q=<script>alert('test');</script> +http://example.com/"<script>alert(''test'');</script> +http://example3.com/test>"'><script>alert('XSS')</script> +http://example.com/"><script>alert('yo')</script> +http://example.com/"><script>alert(''hi'')</script> +localhost<script>alert(''test'')<', 'http://localhost<script>alert(''test'')</script>/test<script>alert(''test'')</script> + +3. go to Piwik UI, and check that in referer everything looks as expected (no parse error, etc.) + |