From d5c16bc178c60403cecccdc8bb2621f577075bf5 Mon Sep 17 00:00:00 2001 From: Torben Hansen Date: Tue, 16 Mar 2021 10:01:42 +0100 Subject: [PATCH] [SECURITY] Prevent urls starting with // to be used for redirects A missing check in GeneralUtility::sanitizeLocalUrl() resulted in an url starting with `//` to be considered as a local url. This change ensures, that urls starting with `//` are not considered local. Corresponding unit tests are fixed and extended, since they need a full environment to process correctly. Resolves: #92891 Releases: master, 11.1, 10.4, 9.5 Change-Id: I41eb16776742b3e0d2cffd064dd0408e4faa7c78 Security-Bulletin: TYPO3-CORE-SA-2021-001 Security-References: CVE-2021-21338 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/68426 Tested-by: Oliver Hader Reviewed-by: Oliver Hader --- .../core/Classes/Utility/GeneralUtility.php | 4 +- .../Tests/Unit/Utility/GeneralUtilityTest.php | 40 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php index 44010ae1ee4f..c48d3dba29bf 100644 --- a/typo3/sysext/core/Classes/Utility/GeneralUtility.php +++ b/typo3/sysext/core/Classes/Utility/GeneralUtility.php @@ -2902,7 +2902,9 @@ public static function sanitizeLocalUrl($url = '') } } elseif (self::isAbsPath($decodedUrl) && self::isAllowedAbsPath($decodedUrl)) { $sanitizedUrl = $url; - } elseif (strpos($testAbsoluteUrl, self::getIndpEnv('TYPO3_SITE_PATH')) === 0 && $decodedUrl[0] === '/') { + } elseif (strpos($testAbsoluteUrl, self::getIndpEnv('TYPO3_SITE_PATH')) === 0 && $decodedUrl[0] === '/' && + substr($decodedUrl, 0, 2) !== '//' + ) { $sanitizedUrl = $url; } elseif (empty($parsedUrl['scheme']) && strpos($testRelativeUrl, self::getIndpEnv('TYPO3_SITE_PATH')) === 0 && $decodedUrl[0] !== '/' && strpbrk($decodedUrl, '*:|"<>') === false && strpos($decodedUrl, '\\\\') === false diff --git a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php index 6dd9bf165cef..f0b562e9fd50 100644 --- a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php +++ b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php @@ -1976,6 +1976,7 @@ public function sanitizeLocalUrlInvalidDataProvider() 'empty string' => [''], 'http domain' => ['http://www.google.de/'], 'https domain' => ['https://www.google.de/'], + 'domain without schema' => ['//www.google.de/'], 'XSS attempt' => ['" onmouseover="alert(123)"'], 'invalid URL, UNC path' => ['\\\\foo\\bar\\'], 'invalid URL, HTML break out attempt' => ['" >blabuubb'], @@ -1984,11 +1985,48 @@ public function sanitizeLocalUrlInvalidDataProvider() } /** + * @param string $url * @test * @dataProvider sanitizeLocalUrlInvalidDataProvider */ - public function sanitizeLocalUrlDeniesPlainInvalidUrls($url) + public function sanitizeLocalUrlDeniesPlainInvalidUrlsInBackendContext(string $url): void { + Environment::initialize( + Environment::getContext(), + true, + false, + Environment::getProjectPath(), + Environment::getPublicPath(), + Environment::getVarPath(), + Environment::getConfigPath(), + Environment::getBackendPath() . '/index.php', + Environment::isWindows() ? 'WINDOWS' : 'UNIX' + ); + $_SERVER['HTTP_HOST'] = 'localhost'; + $_SERVER['SCRIPT_NAME'] = 'typo3/index.php'; + self::assertEquals('', GeneralUtility::sanitizeLocalUrl($url)); + } + + /** + * @param string $url + * @test + * @dataProvider sanitizeLocalUrlInvalidDataProvider + */ + public function sanitizeLocalUrlDeniesPlainInvalidUrlsInFrontendContext(string $url): void + { + Environment::initialize( + Environment::getContext(), + true, + false, + Environment::getProjectPath(), + Environment::getPublicPath(), + Environment::getVarPath(), + Environment::getConfigPath(), + Environment::getPublicPath() . '/index.php', + Environment::isWindows() ? 'WINDOWS' : 'UNIX' + ); + $_SERVER['HTTP_HOST'] = 'localhost'; + $_SERVER['SCRIPT_NAME'] = 'index.php'; self::assertEquals('', GeneralUtility::sanitizeLocalUrl($url)); }