Skip to content

Commit

Permalink
[SECURITY] Prevent urls starting with // to be used for redirects
Browse files Browse the repository at this point in the history
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 <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
derhansen authored and ohader committed Mar 16, 2021
1 parent c887afe commit d5c16bc
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
4 changes: 3 additions & 1 deletion typo3/sysext/core/Classes/Utility/GeneralUtility.php
Expand Up @@ -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
Expand Down
40 changes: 39 additions & 1 deletion typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
Expand Up @@ -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'],
Expand All @@ -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));
}

Expand Down

0 comments on commit d5c16bc

Please sign in to comment.