diff options
| -rw-r--r-- | core/Nonce.php | 2 | ||||
| -rw-r--r-- | core/Url.php | 61 | ||||
| -rw-r--r-- | plugins/Proxy/Controller.php | 2 | ||||
| -rw-r--r-- | tests/core/Url.test.php | 70 |
4 files changed, 104 insertions, 31 deletions
diff --git a/core/Nonce.php b/core/Nonce.php index f50ddc098f..a32c24f017 100644 --- a/core/Nonce.php +++ b/core/Nonce.php @@ -71,7 +71,7 @@ class Piwik_Nonce // validate referer $referer = Piwik_Url::getReferer(); - if(!empty($referer) && (Piwik_Url::getLocalReferer() === false)) + if(!empty($referer) && !Piwik_Url::isLocalUrl($referer)) { return false; } diff --git a/core/Url.php b/core/Url.php index 8fbbf0ac33..169adde642 100644 --- a/core/Url.php +++ b/core/Url.php @@ -330,37 +330,50 @@ class Piwik_Url */ static public function isLocalUrl($url) { + if(empty($url)) + { + return true; + } + // handle case-sensitivity differences - $pathContains = Piwik_Common::isWindows() ? 'stripos' : 'strpos'; + $strcmp = Piwik_Common::isWindows() ? 'strcasecmp' : 'strcmp'; + + $parsedUrl = @parse_url($url); + + $requestUri = isset($_SERVER['SCRIPT_URI']) ? $_SERVER['SCRIPT_URI'] : ''; + $parseRequest = @parse_url($requestUri); - // test the scheme/protocol portion of the reconstructed "current" URL - if(!strncasecmp($url, 'http://', 7) || !strncasecmp($url, 'https://', 8)) + // handle host name mangling + $hosts = array( + $_SERVER['HTTP_HOST'], + self::getCurrentHost(), + ); + if(isset($parseRequest['host'])) { - // determine the offset to begin the comparison - $offset = strpos($url, '://'); - $current = strstr(self::getCurrentUrlWithoutFileName(), '://'); - if($pathContains($url, $current, $offset) === $offset) - { - return true; - } + $hosts[] = $parseRequest['host']; } - return false; - } + // compare scheme/protocol portion of the URL + $scheme = $parsedUrl['scheme']; + if(in_array($scheme, array('http', 'https'))) + { + // compare host + $host = $parsedUrl['host']; + if(in_array($parsedUrl['host'], $hosts)) + { + // compare path (without file name) + $path = isset($parsedUrl['path']) ? str_replace('\\', '/', dirname($parsedUrl['path'].'x')) : '/'; + if(strlen($path) > 1) + { + $path .= '/'; + } - /** - * Get local referrer, i.e., on the same host and in the same script path. - * - * @return string|false - */ - static public function getLocalReferer() - { - // verify that the referrer contains the current URL (minus the filename & query parameters), http://example.org/dir1/dir2/ - $referrer = self::getReferer(); - if($referrer !== false && self::isLocalUrl($referrer)) { - return $referrer; + if(!$strcmp($path, self::getCurrentScriptPath())) + { + return true; + } + } } - return false; } } diff --git a/plugins/Proxy/Controller.php b/plugins/Proxy/Controller.php index bde3a03bee..05ece0b3ce 100644 --- a/plugins/Proxy/Controller.php +++ b/plugins/Proxy/Controller.php @@ -119,7 +119,7 @@ class Piwik_Proxy_Controller extends Piwik_Controller // validate referrer $referrer = Piwik_Url::getReferer(); - if(!empty($referrer) && (Piwik_Url::getLocalReferer() === false)) + if(!empty($referrer) && !Piwik_Url::isLocalUrl($referrer)) { die('Invalid Referer detected - check that your browser sends the Referer header. <br/>The link you would have been redirected to is: '.$url); exit; diff --git a/tests/core/Url.test.php b/tests/core/Url.test.php index e08b04799d..f2534d2c43 100644 --- a/tests/core/Url.test.php +++ b/tests/core/Url.test.php @@ -51,12 +51,32 @@ class Test_Piwik_Url extends UnitTestCase $this->assertEqual(Piwik_Url::getCurrentQueryStringWithParametersModified($parametersNameToValue), ''); } + private function saveGlobals($names) + { + $saved = array(); + foreach($names as $name) + { + $saved[$name] = isset($_SERVER[$name]) ? $_SERVER[$name] : null; + } + return $saved; + } + + private function restoreGlobals($saved) + { + foreach($saved as $name => $value) + { + if($value) + { + $_SERVER[$name] = $value; + } + } + } + public function test_getCurrentHost() { Piwik::createConfigObject(); Zend_Registry::get('config')->setTestEnvironment(); - $saveHttpHost = $_SERVER['HTTP_HOST']; - $saveXFwdHost = isset($_SERVER['HTTP_X_FORWARDED_HOST']) ? $_SERVER['HTTP_X_FORWARDED_HOST'] : null; + $saved = $this->saveGlobals(array('HTTP_HOST', 'HTTP_X_FORWARDED_HOST')); $tests = array( 'localhost IPv4' => array('127.0.0.1', null, null, null, '127.0.0.1'), @@ -87,10 +107,50 @@ class Test_Piwik_Url extends UnitTestCase $this->assertEqual( Piwik_Url::getCurrentHost(), $test[4], $description ); } - $_SERVER['HTTP_HOST'] = $saveHttpHost; - if($saveXFwdHost) + $this->restoreGlobals($saved); + } + + public function test_isLocalUrl() + { + $saved = $this->saveGlobals(array('HTTP_HOST', 'SCRIPT_URI', 'REQUEST_URI')); + + $tests = array( + // simple cases + array('www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true), + array('www.example.com', 'http://www.example.com/path/index.php?module=X', '/path/index.php', 'http://www.example.com/path/', true), + array('www.example.com', 'http://www.example.com/path/', '/path/index.php', 'http://www.example.com/path/index.php?module=Y', true), + array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', 'http://www.example.com/path/?query', true), + + // ignore port + array('www.example.com', 'http://www.example.com:80/path/index.php', '/path/index.php', 'http://www.example.com/path/index.php', true), + array('www.example.com', 'http://www.example.com/path/index.php', '/path/index.php', 'http://www.example.com:80/path/index.php', true), + + // Apache+Rails anomaly in SCRIPT_URI + array('www.example.com', 'http://www.example.com/path/#anchor', 'http://www.example.com/path/index.php', 'http://www.example.com/path/?query', true), + + // mangled HTTP_HOST + array('www.example.com', 'http://example.com/path/#anchor', '/path/index.php', 'http://example.com/path/referrer', true), + + // suppressed Referer + array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', null, true), + array('www.example.com', 'http://www.example.com/path/#anchor', '/path/index.php', '', true), + + // mismatched scheme, host, or path + array('www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'ftp://www.example.com/path/index.php', false), + array('www.example.com', 'http://www.example.com/path/?module=X', '/path/index.php', 'http://example.com/path/index.php', false), + array('www.example.com', 'http://www.example.com/path/', '/path/', 'http://crsf.example.com/path/', false), + array('www.example.com', 'http://www.example.com/path/', '/path/', 'http://www.example.com/path2/', false), + ); + + foreach($tests as $description => $test) { - $_SERVER['HTTP_X_FORWARDED_HOST'] = $saveXFwdHost; + $_SERVER['HTTP_HOST'] = $test[0]; + $_SERVER['SCRIPT_URI'] = $test[1]; + $_SERVER['REQUEST_URI'] = $test[2]; + $urlToTest = $test[3]; + $this->assertEqual( Piwik_Url::isLocalUrl($urlToTest), $test[4], $description ); } + + $this->restoreGlobals($saved); } } |