Index: tests/typo3/sysext/felogin/tx_feloginTest.php =================================================================== --- tests/typo3/sysext/felogin/tx_feloginTest.php (Revision 0) +++ tests/typo3/sysext/felogin/tx_feloginTest.php (Revision 0) @@ -0,0 +1,317 @@ + +* All rights reserved +* +* This script is part of the TYPO3 project. The TYPO3 project is +* free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* The GNU General Public License can be found at +* http://www.gnu.org/copyleft/gpl.html. +* +* This script is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* This copyright notice MUST APPEAR in all copies of the script! +***************************************************************/ + + +require_once t3lib_extMgm::extPath('felogin', 'pi1/class.tx_felogin_pi1.php'); + +/** + * Testcase for URL validation in class tx_felogin_pi1 + * + * @author Helmut Hummel + * @package TYPO3 + * @subpackage tests/typo3/sysext/felogin + */ +class tx_feloginTest extends tx_phpunit_testcase { + + /** + * @var array + */ + private $backupGlobalVariables; + + /** + * @var tx_felogin_pi1 + */ + private $txFelogin; + + /** + * @var string + */ + private $testHostName; + + /** + * @var string + */ + private $testSitePath; + + /** + * @var string + */ + private $testTableName; + + public function setUp() { + $this->backupGlobalVariables = array( + '_SERVER' => $_SERVER, + 'TYPO3_DB' => $GLOBALS['TYPO3_DB'], + 'TSFE' => $GLOBALS['TSFE'], + ); + + $this->testTableName = 'sys_domain'; + $this->testHostName = 'hostname.tld'; + $this->testSitePath = '/'; + + // we need to subclass because the method we want to test is protected + $className = uniqid('FeLogin_'); + eval(' + class ' . $className. ' extends tx_felogin_pi1 { + public function validateRedirectUrl($url) { + return parent::validateRedirectUrl($url); + } + } + '); + + $this->txFelogin = new $className(); + $this->txFelogin->cObj = $this->getMock('tslib_cObj'); + + $this->setUpTSFE(); + $this->setUpFakeSitePathAndHost(); + } + + private function setUpTSFE() { + $GLOBALS['TSFE'] = $this->getMock('tslib_fe', array(), array(), '', FALSE); + } + + private function setUpFakeSitePathAndHost() { + $_SERVER['ORIG_PATH_INFO'] = + $_SERVER['PATH_INFO'] = + $_SERVER['ORIG_SCRIPT_NAME'] = + $_SERVER['SCRIPT_NAME'] = $this->testSitePath . TYPO3_mainDir; + $_SERVER['HTTP_HOST'] = $this->testHostName; + } + + private function setUpDatabaseMock() { + $GLOBALS['TYPO3_DB'] = $this->getMock('t3lib_DB', array('exec_SELECTgetRows')); + $GLOBALS['TYPO3_DB']->expects($this->any())->method('exec_SELECTgetRows')->will( + $this->returnCallback(array($this, 'getDomainRecordsCallback')) + ); + } + + /** + * Callback method for pageIdCanBeDetermined test cases. + * Simulates TYPO3_DB->exec_SELECTgetRows(). + * + * @param string $fields + * @param string $table + * @param string $where + * @return mixed + * @see setUpDatabaseMock + */ + public function getDomainRecordsCallback($fields, $table, $where) { + + if ($table !== $this->testTableName) { + return FALSE; + } + + return array( + array('domainName' => 'domainhostname.tld'), + array('domainName' => 'otherhostname.tld/path'), + array('domainName' => 'sub.domainhostname.tld/path/') + ); + } + + public function tearDown() { + $this->txFelogin = null; + + foreach ($this->backupGlobalVariables as $key => $data) { + $GLOBALS[$key] = $data; + } + + $this->backupGlobalVariables = null; + + } + + /** + * @test + */ + public function typo3SitePathEqualsStubSitePath() { + $this->assertEquals(t3lib_div::getIndpEnv('TYPO3_SITE_PATH'), $this->testSitePath); + } + + /** + * @test + */ + public function typo3SiteUrlEqualsStubSiteUrl() { + $this->assertEquals(t3lib_div::getIndpEnv('TYPO3_SITE_URL'), 'http://' . $this->testHostName . $this->testSitePath); + } + + /** + * @test + */ + public function typo3SitePathEqualsStubSitePathAfterChangingInTest() { + $this->testHostName = 'somenewhostname.com'; + $this->testSitePath = '/somenewpath/'; + $this->setUpFakeSitePathAndHost(); + + $this->assertEquals(t3lib_div::getIndpEnv('TYPO3_SITE_PATH'), $this->testSitePath); + } + + /** + * @test + */ + public function typo3SiteUrlEqualsStubSiteUrlAfterChangingInTest() { + $this->testHostName = 'somenewhostname.com'; + $this->testSitePath = '/somenewpath/'; + $this->setUpFakeSitePathAndHost(); + + $this->assertEquals(t3lib_div::getIndpEnv('TYPO3_SITE_URL'), 'http://' . $this->testHostName . $this->testSitePath); + } + + /** + * Data provider for malicousUrlsWillBeCleared + * + * @see malicousUrlsWillBeCleared + */ + public function variousUrlsDataProviderForMalicousUrlsWillBeCleared() { + return array( + 'absolute URL, hostname not in sys_domain, trailing slash' => array('http://badhost.tld/'), + 'absolute URL, hostname not in sys_domain, no trailing slash' => array('http://badhost.tld'), + 'absolute URL, subdomain in sys_domain, but main domain not, trailing slash' => array('http://domainhostname.tld.badhost.tld/'), + 'absolute URL, subdomain in sys_domain, but main domain not, no trailing slash' => array('http://domainhostname.tld.badhost.tld'), + 'non http absolute URL 1' => array('its://domainhostname.tld/itunes/'), + 'non http absolute URL 2' => array('ftp://domainhostname.tld/download/'), + + 'XSS attempt 1' => array('javascript:alert(123)'), + 'XSS attempt 2' => array('" onmouseover="alert(123)"'), + 'invalid URL, HTML break out attempt' => array('" >blabuubb'), + 'invalid URL, UNC path' => array('\\\\foo\\bar\\'), + 'invalid URL, backslashes in path' => array('http://domainhostname.tld\\bla\\blupp'), + 'invalid URL, linefeed in path' => array("http://domainhostname.tld/bla/blupp\n"), + 'invalid URL, only one slash after scheme' => array("http:/domainhostname.tld/bla/blupp"), + 'invalid URL, illegal chars' => array("http://(<>domainhostname).tld/bla/blupp"), + + ); + } + + /** + * @test + * @dataProvider variousUrlsDataProviderForMalicousUrlsWillBeCleared + */ + public function malicousUrlsWillBeCleared($url) { + $this->setUpDatabaseMock(); + $this->assertEquals('', $this->txFelogin->validateRedirectUrl($url)); + } + + /** + * Data provider for cleanUrlsAreKept + * + * @see cleanUrlsAreKept + */ + public function variousUrlsDataProviderForCleanUrlsAreKept() { + return array( + 'sane absolute URL' => array('http://domainhostname.tld/'), + 'sane absolute URL with script' => array('http://domainhostname.tld/index.php?id=1'), + 'sane absolute URL with realurl' => array('http://domainhostname.tld/foo/bar/foo.html'), + 'sane absolute URL with homedir' => array('http://domainhostname.tld/~user/'), + 'sane absolute URL with some strange chars encoded' => array('http://domainhostname.tld/~user/a%cc%88o%cc%88%c3%9fa%cc%82/foo.html'), + + 'sane absolute URL (domain record with path)' => array('http://otherhostname.tld/path/'), + 'sane absolute URL with script (domain record with path)' => array('http://otherhostname.tld/path/index.php?id=1'), + 'sane absolute URL with realurl (domain record with path)' => array('http://otherhostname.tld/path/foo/bar/foo.html'), + + 'sane absolute URL (domain record with path and slash)' => array('http://sub.domainhostname.tld/path/'), + 'sane absolute URL with script (domain record with path slash)' => array('http://sub.domainhostname.tld/path/index.php?id=1'), + 'sane absolute URL with realurl (domain record with path slash)' => array('http://sub.domainhostname.tld/path/foo/bar/foo.html'), + + 'relative URL, no leading slash 1' => array('index.php?id=1'), + 'relative URL, no leading slash 2' => array('foo/bar/index.php?id=2'), + 'relative URL, leading slash, no realurl' => array('/index.php?id=1'), + 'relative URL, leading slash, realurl' => array('/de/service/imprint.html'), + ); + } + + /** + * @test + * @dataProvider variousUrlsDataProviderForCleanUrlsAreKept + */ + public function cleanUrlsAreKept($url) { + $this->setUpDatabaseMock(); + $this->assertEquals($url, $this->txFelogin->validateRedirectUrl($url)); + } + + /** + * Data provider for malicousUrlsWillBeClearedTypo3InSubdirectory + * + * @see malicousUrlsWillBeClearedTypo3InSubdirectory + */ + public function variousUrlsDataProviderForMalicousUrlsWillBeClearedTypo3InSubdirectory() { + return array( + 'absolute URL, missing subdirectory' => array('http://hostname.tld/'), + 'absolute URL, wrong subdirectory' => array('http://hostname.tld/hacker/index.php'), + 'absolute URL, correct subdirectory, no trailing slash' => array('http://hostname.tld/subdir'), + 'absolute URL, correct subdirectory of sys_domain record, no trailing slash' => array('http://otherhostname.tld/path'), + 'absolute URL, correct subdirectory of sys_domain record, no trailing slash' => array('http://sub.domainhostname.tld/path'), + + 'relative URL, leading slash, no path' => array('/index.php?id=1'), + 'relative URL, leading slash, wrong path' => array('/de/sub/site.html'), + 'relative URL, leading slash, slash only' => array('/'), + ); + } + + /** + * @test + * @dataProvider variousUrlsDataProviderForMalicousUrlsWillBeClearedTypo3InSubdirectory + */ + public function malicousUrlsWillBeClearedTypo3InSubdirectory($url) { + $this->testSitePath = '/subdir/'; + $this->setUpFakeSitePathAndHost(); + + $this->setUpDatabaseMock(); + $this->assertEquals('', $this->txFelogin->validateRedirectUrl($url)); + } + + /** + * Data provider for cleanUrlsAreKeptTypo3InSubdirectory + * + * @see cleanUrlsAreKeptTypo3InSubdirectory + */ + public function variousUrlsDataProviderForCleanUrlsAreKeptTypo3InSubdirectory() { + return array( + 'absolute URL, correct subdirectory' => array('http://hostname.tld/subdir/'), + 'absolute URL, correct subdirectory, realurl' => array('http://hostname.tld/subdir/de/imprint.html'), + 'absolute URL, correct subdirectory, no realurl' => array('http://hostname.tld/subdir/index.php?id=10'), + + 'absolute URL, correct subdirectory of sys_domain record' => array('http://otherhostname.tld/path/'), + 'absolute URL, correct subdirectory of sys_domain record' => array('http://sub.domainhostname.tld/path/'), + + 'relative URL, no leading slash, realurl' => array('de/service/imprint.html'), + 'relative URL, no leading slash, no realurl' => array('index.php?id=1'), + 'relative URL, no leading slash, no realurl' => array('foo/bar/index.php?id=2'), + ); + } + + /** + * @test + * @dataProvider variousUrlsDataProviderForCleanUrlsAreKeptTypo3InSubdirectory + */ + public function cleanUrlsAreKeptTypo3InSubdirectory($url) { + $this->testSitePath = '/subdir/'; + $this->setUpFakeSitePathAndHost(); + + $this->setUpDatabaseMock(); + $this->assertEquals($url, $this->txFelogin->validateRedirectUrl($url)); + } + + + +} +?> \ No newline at end of file Index: typo3/sysext/felogin/pi1/class.tx_felogin_pi1.php =================================================================== --- typo3/sysext/felogin/pi1/class.tx_felogin_pi1.php (Revision 8481) +++ typo3/sysext/felogin/pi1/class.tx_felogin_pi1.php (Arbeitskopie) @@ -111,8 +111,8 @@ $this->redirectUrl = $this->conf['redirectFirstMethod'] ? array_shift($redirectUrl) : array_pop($redirectUrl); } else { $this->redirectUrl = ''; + } } - } // What to display $content=''; @@ -871,20 +871,17 @@ return ''; } - $sanitizedUrl = t3lib_div::removeXSS(rawurldecode($url)); - if ($url !== $sanitizedUrl) { + $decodedUrl = rawurldecode($url); + $sanitizedUrl = t3lib_div::removeXSS($decodedUrl); + + if ($decodedUrl !== $sanitizedUrl || preg_match('#["<>\\\]+#', $url)) { t3lib_div::sysLog(sprintf($this->pi_getLL('xssAttackDetected'), $url), 'felogin', t3lib_div::SYSLOG_SEVERITY_WARNING); return ''; } - if (!t3lib_div::isValidUrl($sanitizedUrl)) { - t3lib_div::sysLog(sprintf($this->pi_getLL('noValidRedirectUrl'), $sanitizedUrl), 'felogin', t3lib_div::SYSLOG_SEVERITY_WARNING); - return ''; - } - // Validate the URL: - if ($this->isInCurrentDomain($sanitizedUrl) || $this->isInLocalDomain($sanitizedUrl)) { - return $sanitizedUrl; + if ($this->isRelativeUrl($url) || $this->isInCurrentDomain($url) || $this->isInLocalDomain($url)) { + return $url; } // URL is not allowed @@ -900,39 +897,61 @@ * @return boolean Whether the URL belongs to the current TYPO3 installation */ protected function isInCurrentDomain($url) { - return (t3lib_div::isOnCurrentHost($url) AND strpos($url, t3lib_div::getIndpEnv('TYPO3_SITE_URL')) === 0); + return (t3lib_div::isOnCurrentHost($url) && t3lib_div::isFirstPartOfStr($url, t3lib_div::getIndpEnv('TYPO3_SITE_URL'))); } /** * Determines whether the URL matches a domain * in the sys_domain databse table. * - * @param string $domain Name of the domain to be looked up - * @return boolean Whether the domain name is considered to be local + * @param string $url Absolute URL which needs to be checked + * @return boolean Whether the URL is considered to be local */ protected function isInLocalDomain($url) { $result = FALSE; - $parsedUrl = parse_url($url); - $domain = $parsedUrl['host'] . $parsedUrl['path']; + if (t3lib_div::isValidUrl($url)) { + $parsedUrl = parse_url($url); + if ($parsedUrl['scheme'] === 'http' || $parsedUrl['scheme'] === 'https' ) { + $host = $parsedUrl['host']; + // Removes the last path segment and slash sequences like /// (if given): + $path = preg_replace('#/+[^/]*$#', '', $parsedUrl['path']); - $localDomains = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows( - 'domainName', - 'sys_domain', - '1=1' . $this->cObj->enableFields('sys_domain') - ); - - if (is_array($localDomains)) { - foreach ($localDomains as $localDomain) { - if (stripos($domain, $localDomain['domainName']) === 0) { - $result = TRUE; - break; + $localDomains = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows( + 'domainName', + 'sys_domain', + '1=1' . $this->cObj->enableFields('sys_domain') + ); + if (is_array($localDomains)) { + foreach ($localDomains as $localDomain) { + // strip trailing slashes (if given) + $domainName = rtrim($localDomain['domainName'], '/'); + if (t3lib_div::isFirstPartOfStr($host. $path . '/', $domainName . '/')) { + $result = TRUE; + break; + } + } } } } - return $result; } + + /** + * Determines wether the URL is relative to the + * current TYPO3 installation. + * + * @param string $url URL which needs to be checked + * @return boolean Whether the URL is considered to be relative + */ + protected function isRelativeUrl($url) { + $parsedUrl = @parse_url($url); + if ($parsedUrl !== FALSE && !isset($parsedUrl['scheme']) && !isset($parsedUrl['host'])) { + // If the relative URL starts with a slash, we need to check if it's within the current site path + return (!t3lib_div::isFirstPartOfStr($parsedUrl['path'], '/') || t3lib_div::isFirstPartOfStr($parsedUrl['path'], t3lib_div::getIndpEnv('TYPO3_SITE_PATH'))); + } + return FALSE; + } }