From fbafe16c48fa47fd8d5d4d66436700b8d85d1bfa Mon Sep 17 00:00:00 2001 From: Oliver Hader <oliver@typo3.org> Date: Thu, 14 May 2020 16:47:04 +0200 Subject: [PATCH] [BUGFIX] Allow multiple referrer types in backend main route With TYPO3-CORE-SA-2020-006 (SSRF via XSS) a strict referrer handling has been introduced to avoid the TYPO3 backend being called from other non same-origin locations. In case a HTTP referrer header was empty the system tried to refresh the view - otherwise the request was denied completely. It turned out that this scenario was probably too strict, disabling feature `security.backend.enforceReferrer` was the only work-around for site administrators. This change adds new options for handling referrers in backend routes: * refresh-empty (existed already): refresh in case referrer is empty * refresh-same-site: refresh in case referrer is on same site, like `https://example.org/?eID=auth` calling `https://example.org/typo3/` * refresh-always: refresh always in case there is not valid referrer TYPO3's main backend route is using `refresh-always` now to be more relaxed on handling same-site and cross-site referrers as well. The term "refreshing" relates to trigger a reload in the browser to get the referrer of the current location. This still block direct CSRF/SSRF requests since the refreshing HTML instructions are delivered back to the client. Besides that, cross-site requests are covered by the `same-site` cookie policy, and existing CSRF tokens. Resolves: #91396 Releases: master, 9.5 Change-Id: Ib3756671fa60c6f41ba992d0e645f03da1730d19 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64492 Tested-by: Susanne Moog <look@susi.dev> Tested-by: TYPO3com <noreply@typo3.com> Tested-by: Richard Haeser <richard@maxserv.com> Reviewed-by: Susanne Moog <look@susi.dev> Reviewed-by: Richard Haeser <richard@maxserv.com> --- .../backend/Configuration/Backend/Routes.php | 2 +- .../Http/Security/ReferrerEnforcer.php | 61 +++++- .../Http/Security/ReferrerEnforcerTest.php | 183 ++++++++++++++++++ 3 files changed, 238 insertions(+), 8 deletions(-) create mode 100644 typo3/sysext/core/Tests/Unit/Http/Security/ReferrerEnforcerTest.php diff --git a/typo3/sysext/backend/Configuration/Backend/Routes.php b/typo3/sysext/backend/Configuration/Backend/Routes.php index 9e3978d0494e..5e5f4553822d 100644 --- a/typo3/sysext/backend/Configuration/Backend/Routes.php +++ b/typo3/sysext/backend/Configuration/Backend/Routes.php @@ -23,7 +23,7 @@ return [ // Main backend rendering setup (previously called backend.php) for the TYPO3 Backend 'main' => [ 'path' => '/main', - 'referrer' => 'required,refresh-empty', + 'referrer' => 'required,refresh-always', 'target' => Controller\BackendController::class . '::mainAction' ], diff --git a/typo3/sysext/core/Classes/Http/Security/ReferrerEnforcer.php b/typo3/sysext/core/Classes/Http/Security/ReferrerEnforcer.php index f4497c654879..5a7b840d4372 100644 --- a/typo3/sysext/core/Classes/Http/Security/ReferrerEnforcer.php +++ b/typo3/sysext/core/Classes/Http/Security/ReferrerEnforcer.php @@ -29,11 +29,20 @@ use TYPO3\CMS\Core\Utility\PathUtility; */ class ReferrerEnforcer { + private const TYPE_REFERRER_EMPTY = 1; + private const TYPE_REFERRER_SAME_SITE = 2; + private const TYPE_REFERRER_SAME_ORIGIN = 4; + /** * @var ServerRequestInterface */ protected $request; + /** + * @var string + */ + protected $requestHost; + /** * @var string */ @@ -42,14 +51,15 @@ class ReferrerEnforcer public function __construct(ServerRequestInterface $request) { $this->request = $request; + $this->requestHost = rtrim($this->resolveRequestHost($request), '/') . '/'; $this->requestDir = $this->resolveRequestDir($request); } public function handle(array $options = null): ?ResponseInterface { - $referrer = $this->request->getServerParams()['HTTP_REFERER'] ?? ''; + $referrerType = $this->resolveReferrerType(); // valid referrer, no more actions required - if ($referrer !== '' && strpos($referrer, $this->requestDir) === 0) { + if ($referrerType & self::TYPE_REFERRER_SAME_ORIGIN) { return null; } $flags = $options['flags'] ?? []; @@ -57,13 +67,18 @@ class ReferrerEnforcer // referrer is missing and route requested to refresh // (created HTML refresh to enforce having referrer) if (($this->request->getQueryParams()['referrer-refresh'] ?? 0) <= time() - && $referrer === '' && in_array('refresh-empty', $flags, true)) { + && ( + in_array('refresh-always', $flags, true) + || ($referrerType & self::TYPE_REFERRER_EMPTY && in_array('refresh-empty', $flags, true)) + || ($referrerType & self::TYPE_REFERRER_SAME_SITE && in_array('refresh-same-site', $flags, true)) + ) + ) { $refreshUri = $this->request->getUri(); $refreshUri = $refreshUri->withQuery( $refreshUri->getQuery() . '&referrer-refresh=' . (time() + $expiration) ); - $scriptUri = PathUtility::getAbsoluteWebPath( - GeneralUtility::getFileAbsFileName('EXT:core/Resources/Public/JavaScript/ReferrerRefresh.js') + $scriptUri = $this->resolveAbsoluteWebPath( + 'EXT:core/Resources/Public/JavaScript/ReferrerRefresh.js' ); // simulating navigate event by clicking anchor link // since meta-refresh won't change `document.referrer` in e.g. Firefox @@ -78,7 +93,7 @@ class ReferrerEnforcer )); } $subject = $options['subject'] ?? ''; - if ($referrer === '') { + if ($referrerType & self::TYPE_REFERRER_EMPTY) { // still empty referrer or invalid referrer, deny route invocation throw new MissingReferrerException( sprintf('Missing referrer%s', $subject !== '' ? ' for ' . $subject : ''), @@ -87,11 +102,43 @@ class ReferrerEnforcer } // referrer is given, but does not match current base URL throw new InvalidReferrerException( - sprintf('Missing referrer%s', $subject !== '' ? ' for ' . $subject : ''), + sprintf('Invalid referrer%s', $subject !== '' ? ' for ' . $subject : ''), 1588095936 ); } + protected function resolveAbsoluteWebPath(string $target): string + { + return PathUtility::getAbsoluteWebPath( + GeneralUtility::getFileAbsFileName($target) + ); + } + + protected function resolveReferrerType(): int + { + $referrer = $this->request->getServerParams()['HTTP_REFERER'] ?? ''; + if ($referrer === '') { + return self::TYPE_REFERRER_EMPTY; + } + if (strpos($referrer, $this->requestDir) === 0) { + // same-origin implies same-site + return self::TYPE_REFERRER_SAME_ORIGIN | self::TYPE_REFERRER_SAME_SITE; + } + if (strpos($referrer, $this->requestHost) === 0) { + return self::TYPE_REFERRER_SAME_SITE; + } + return 0; + } + + protected function resolveRequestHost(ServerRequestInterface $request): string + { + $normalizedParams = $request->getAttribute('normalizedParams'); + if ($normalizedParams instanceof NormalizedParams) { + return $normalizedParams->getRequestHost(); + } + return GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST'); + } + protected function resolveRequestDir(ServerRequestInterface $request): string { $normalizedParams = $request->getAttribute('normalizedParams'); diff --git a/typo3/sysext/core/Tests/Unit/Http/Security/ReferrerEnforcerTest.php b/typo3/sysext/core/Tests/Unit/Http/Security/ReferrerEnforcerTest.php new file mode 100644 index 000000000000..aee48a75638c --- /dev/null +++ b/typo3/sysext/core/Tests/Unit/Http/Security/ReferrerEnforcerTest.php @@ -0,0 +1,183 @@ +<?php + +declare(strict_types=1); + +/* + * This file is part of the TYPO3 CMS project. + * + * It is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License, either version 2 + * of the License, or any later version. + * + * For the full copyright and license information, please read the + * LICENSE.txt file that was distributed with this source code. + * + * The TYPO3 project - inspiring people to share! + */ + +namespace TYPO3\CMS\Core\Tests\Unit\Http\Security; + +use Prophecy\Prophecy\ObjectProphecy; +use Psr\Http\Message\ServerRequestInterface; +use TYPO3\CMS\Core\Http\NormalizedParams; +use TYPO3\CMS\Core\Http\Security\InvalidReferrerException; +use TYPO3\CMS\Core\Http\Security\MissingReferrerException; +use TYPO3\CMS\Core\Http\Security\ReferrerEnforcer; +use TYPO3\CMS\Core\Http\Uri; +use TYPO3\TestingFramework\Core\Unit\UnitTestCase; + +class ReferrerEnforcerTest extends UnitTestCase +{ + private static function buildRefreshContentPattern(string $uri): string + { + return sprintf( + '#.+href="%s\d+" id="referrer-refresh".+#', + preg_quote(htmlspecialchars($uri . '&referrer-refresh='), '#') + ); + } + + public function validReferrerIsHandledDataProvider(): array + { + return [ + [ + 'https://example.org/typo3/index.php?route=%2Flogin', // requestUri + 'https://example.org/typo3/index.php', // referrer + null, // options + null, // response + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + '', + ['flags' => ['refresh-empty']], + self::buildRefreshContentPattern( + 'https://example.org/typo3/index.php?route=%2Flogin' + ), + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + 'https://example.org/?eID=handler', + ['flags' => ['refresh-same-site']], + self::buildRefreshContentPattern( + 'https://example.org/typo3/index.php?route=%2Flogin' + ), + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + 'https://other-example.site/security/', + ['flags' => ['refresh-always']], + self::buildRefreshContentPattern( + 'https://example.org/typo3/index.php?route=%2Flogin' + ), + ], + ]; + } + + /** + * @param string $requestUri + * @param string $referrer + * @param string[]|null $options + * @param string|null $expectedResponse + * + * @test + * @dataProvider validReferrerIsHandledDataProvider + */ + public function validReferrerIsHandled(string $requestUri, string $referrer, ?array $options, ?string $expectedResponse): void + { + $subject = $this->buildSubject($requestUri, $referrer); + $response = $subject->handle($options); + + if ($expectedResponse === null) { + self::assertNull($response); + } else { + self::assertRegExp($expectedResponse, (string)$response->getBody()); + } + } + + public function invalidReferrerIsHandledDataProvider(): array + { + return [ + [ + 'https://example.org/typo3/index.php?route=%2Flogin', // requestUri + 'https://example.org/?eID=handler', // referrer + null, // options + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + 'https://example.org/?eID=handler', + ['flags' => ['refresh-empty']], + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + 'https://example.org.security/?eID=handler', + ['flags' => ['refresh-same-site']], + ], + [ + 'https://example.org/typo3/index.php?route=%2Flogin', + 'https://other-example.site/security/', + null, + ], + ]; + } + + /** + * @param string $requestUri + * @param string $referrer + * @param string[]|null $options + * + * @test + * @dataProvider invalidReferrerIsHandledDataProvider + */ + public function invalidReferrerIsHandled(string $requestUri, string $referrer, ?array $options): void + { + $this->expectException(InvalidReferrerException::class); + $this->expectExceptionCode(1588095936); + $subject = $this->buildSubject($requestUri, $referrer); + $subject->handle($options); + } + + /** + * @test + */ + public function missingReferrerIsHandled(): void + { + $this->expectException(MissingReferrerException::class); + $this->expectExceptionCode(1588095935); + $subject = $this->buildSubject( + 'https://example.org/typo3/index.php?route=%2Flogin', + '' + ); + $subject->handle(); + } + + private function buildSubject(string $requestUri, string $referrer): ReferrerEnforcer + { + $requestUriInstance = new Uri($requestUri); + $host = sprintf( + '%s://%s', + $requestUriInstance->getScheme(), + $requestUriInstance->getHost() + ); + $dir = $host . rtrim(dirname($requestUriInstance->getPath()), '/') . '/'; + parse_str($requestUriInstance->getQuery(), $queryParams); + + /** @var NormalizedParams|ObjectProphecy $normalizedParams */ + $normalizedParams = $this->prophesize(NormalizedParams::class); + $normalizedParams->getRequestHost()->willReturn($host); + $normalizedParams->getRequestDir()->willReturn($dir); + /** @var ServerRequestInterface|ObjectProphecy $request */ + $request = $this->prophesize(ServerRequestInterface::class); + $request->getAttribute('normalizedParams')->willReturn($normalizedParams); + $request->getServerParams()->willReturn(['HTTP_REFERER' => $referrer]); + $request->getUri()->willReturn($requestUriInstance); + $request->getQueryParams()->willReturn($queryParams); + + $subject = $this->getMockBuilder(ReferrerEnforcer::class) + ->setConstructorArgs([$request->reveal()]) + ->onlyMethods(['resolveAbsoluteWebPath']) + ->getMock(); + $subject->method('resolveAbsoluteWebPath') + ->with('EXT:core/Resources/Public/JavaScript/ReferrerRefresh.js') + ->willReturn('/typo3/sysext/core/Resources/Public/JavaScript/ReferrerRefresh.js'); + return $subject; + } +} -- GitLab