From 71e2d04ee8e786d3deb8dfa2c323e571e19c95e9 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 25 Jul 2023 10:03:28 +0200
Subject: [PATCH] [SECURITY] Avoid out-of-scope page access for non-matching
site
This change disallows calling an URI with page-id query parameters
that are not part of a particular site - for instance the following
URL `https://example.org/?id=3000&L=0` has two aspects:
* the site `example.org` has the root page-id 1000
* the site `internal.example.org` has the root page-id 3000
The example above allows to call a page-id for an internal site,
by using a valid and public entry point.
The new feature flag
`security.frontend.allowInsecureSiteResolutionByQueryParameters`
allows to control this behavior for backward compatibility reasons.
Per default `allowInsecureSiteResolutionByQueryParameters` is disabled.
Resolves: #100889
Releases: main, 12.4, 11.5
Change-Id: I88d565b5d9bea556b4f754c3069d56124cea98bd
Security-Bulletin: TYPO3-CORE-SA-2023-003
Security-References: CVE-2023-38499
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80156
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
.../core/Classes/Routing/SiteMatcher.php | 195 ++++++++++++------
.../core/Classes/Routing/SiteRouteResult.php | 11 +
.../Configuration/DefaultConfiguration.php | 1 +
.../DefaultConfigurationDescription.yaml | 3 +
.../Tests/Unit/Routing/SiteMatcherTest.php | 20 ++
...esolutionByQueryParametersDisabledTest.php | 116 +++++++++++
...ResolutionByQueryParametersEnabledTest.php | 118 +++++++++++
.../SiteHandling/SlugSiteRequestTest.php | 6 +
.../Unit/Middleware/SiteResolverTest.php | 21 ++
9 files changed, 423 insertions(+), 68 deletions(-)
create mode 100644 typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest.php
create mode 100644 typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest.php
diff --git a/typo3/sysext/core/Classes/Routing/SiteMatcher.php b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
index 3428ef9a252e..177ffd442304 100644
--- a/typo3/sysext/core/Classes/Routing/SiteMatcher.php
+++ b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
@@ -18,10 +18,12 @@ declare(strict_types=1);
namespace TYPO3\CMS\Core\Routing;
use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Message\UriInterface;
use Symfony\Component\Routing\Exception\NoConfigurationException;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\RequestContext;
use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Configuration\Features;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
use TYPO3\CMS\Core\Http\NormalizedParams;
use TYPO3\CMS\Core\SingletonInterface;
@@ -89,84 +91,42 @@ class SiteMatcher implements SingletonInterface
*/
public function matchRequest(ServerRequestInterface $request): RouteResultInterface
{
- $site = new NullSite();
- $language = null;
- $defaultLanguage = null;
+ // Remove script file name (index.php) from request uri
+ $uri = $this->canonicalizeUri($request->getUri(), $request);
+ $pageId = $this->resolvePageIdQueryParam($request);
+ $languageId = $this->resolveLanguageIdQueryParam($request);
- $pageId = $request->getQueryParams()['id'] ?? $request->getParsedBody()['id'] ?? 0;
+ $routeResult = $this->matchSiteByUri($uri, $request);
- // First, check if we have a _GET/_POST parameter for "id", then a site information can be resolved based.
- if ($pageId > 0) {
- // Loop over the whole rootline without permissions to get the actual site information
- try {
- $site = $this->finder->getSiteByPageId((int)$pageId);
- // If a "L" parameter is given, we take that one into account.
- $languageId = $request->getQueryParams()['L'] ?? $request->getParsedBody()['L'] ?? null;
- if ($languageId !== null) {
- $language = $site->getLanguageById((int)$languageId);
- } else {
- // Use this later below
- $defaultLanguage = $site->getDefaultLanguage();
- }
- } catch (SiteNotFoundException $e) {
- // No site found by the given page
- } catch (\InvalidArgumentException $e) {
- // The language fetched by getLanguageById() was not available, now the PSR-15 middleware
- // redirects to the default page.
- }
+ // Allow insecure pageId based site resolution if explicitly enabled and only if both, ?id= and ?L= are defined
+ // (pageId based site resolution without L parameter has always been prohibited, so we do not support that)
+ if (
+ GeneralUtility::makeInstance(Features::class)->isFeatureEnabled('security.frontend.allowInsecureSiteResolutionByQueryParameters') &&
+ $pageId !== null && $languageId !== null
+ ) {
+ return $this->matchSiteByQueryParams($pageId, $languageId, $routeResult, $uri);
}
- $uri = $request->getUri();
- if (!empty($uri->getPath())) {
- $normalizedParams = $request->getAttribute('normalizedParams');
- if ($normalizedParams instanceof NormalizedParams) {
- $urlPath = ltrim($uri->getPath(), '/');
- $scriptName = ltrim($normalizedParams->getScriptName(), '/');
- $scriptPath = ltrim($normalizedParams->getSitePath(), '/');
- if ($scriptName !== '' && str_starts_with($urlPath, $scriptName)) {
- $urlPath = '/' . $scriptPath . substr($urlPath, mb_strlen($scriptName));
- $uri = $uri->withPath($urlPath);
- }
- }
+ // Allow the default language to be resolved in case all languages use a prefix
+ // and therefore did not match based on path if an explicit pageId is given,
+ // (example "https://www.example.com/?id=.." was entered, but all languages have "https://www.example.com/lang-key/")
+ // @todo remove this fallback, in order for SiteBaseRedirectResolver to produce a redirect instead (requires functionals to be adapted)
+ if ($pageId !== null && $routeResult->getLanguage() === null) {
+ $routeResult = $routeResult->withLanguage($routeResult->getSite()->getDefaultLanguage());
}
- // No language found at this point means that the URL was not used with a valid "?id=1&L=2" parameter
- // which resulted in a site / language combination that was found. Now, the matching is done
- // on the incoming URL.
- if (!($language instanceof SiteLanguage)) {
- $collection = $this->getRouteCollectionForAllSites();
- $context = new RequestContext(
- '',
- $request->getMethod(),
- (string)idn_to_ascii($uri->getHost()),
- $uri->getScheme(),
- // Ports are only necessary for URL generation in Symfony which is not used by TYPO3
- 80,
- 443,
- $uri->getPath()
- );
- $matcher = new BestUrlMatcher($collection, $context);
+ // adjust the language aspect if it was given by query param `&L` (and ?id is given)
+ // @todo remove, this is added for backwards (and functional tests) compatibility reasons
+ if ($languageId !== null && $pageId !== null) {
try {
- $result = $matcher->match($uri->getPath());
- return new SiteRouteResult(
- $uri,
- $result['site'],
- // if no language is found, this usually results due to "/" called instead of "/fr/"
- // but it could also be the reason that "/index.php?id=23" was called, so the default
- // language is used as a fallback here then.
- $result['language'] ?? $defaultLanguage,
- $result['tail']
- );
- } catch (NoConfigurationException | ResourceNotFoundException $e) {
- // At this point we discard a possible found site via ?id=123
- // Because ?id=123 _can_ only work if the actual domain/site base works
- // so www.domain-without-site-configuration/index.php?id=123 (where 123 is a page referring
- // to a page within a site configuration will never be resolved here) properly
- $site = new NullSite();
+ // override/set language by `&L=` query param
+ $routeResult = $routeResult->withLanguage($routeResult->getSite()->getLanguageById($languageId));
+ } catch (\InvalidArgumentException $e) {
+ // ignore; language id not available
}
}
- return new SiteRouteResult($uri, $site, $language);
+ return $routeResult;
}
/**
@@ -229,4 +189,103 @@ class SiteMatcher implements SingletonInterface
}
return $collection;
}
+
+ /**
+ * @return ?positive-int
+ */
+ protected function resolvePageIdQueryParam(ServerRequestInterface $request): ?int
+ {
+ $pageId = $request->getQueryParams()['id'] ?? $request->getParsedBody()['id'] ?? null;
+ if ($pageId === null) {
+ return null;
+ }
+ return (int)$pageId <= 0 ? null : (int)$pageId;
+ }
+
+ /**
+ * @return ?positive-int
+ */
+ protected function resolveLanguageIdQueryParam(ServerRequestInterface $request): ?int
+ {
+ $languageId = $request->getQueryParams()['L'] ?? $request->getParsedBody()['L'] ?? null;
+ if ($languageId === null) {
+ return null;
+ }
+ return (int)$languageId < 0 ? null : (int)$languageId;
+ }
+
+ /**
+ * Remove script file name (index.php) from request uri
+ */
+ protected function canonicalizeUri(UriInterface $uri, ServerRequestInterface $request): UriInterface
+ {
+ if ($uri->getPath() === '') {
+ return $uri;
+ }
+
+ $normalizedParams = $request->getAttribute('normalizedParams');
+ if (!$normalizedParams instanceof NormalizedParams) {
+ return $uri;
+ }
+
+ $urlPath = ltrim($uri->getPath(), '/');
+ $scriptName = ltrim($normalizedParams->getScriptName(), '/');
+ $scriptPath = ltrim($normalizedParams->getSitePath(), '/');
+ if ($scriptName !== '' && str_starts_with($urlPath, $scriptName)) {
+ $urlPath = '/' . $scriptPath . substr($urlPath, mb_strlen($scriptName));
+ $uri = $uri->withPath($urlPath);
+ }
+
+ return $uri;
+ }
+
+ protected function matchSiteByUri(UriInterface $uri, ServerRequestInterface $request): SiteRouteResult
+ {
+ $collection = $this->getRouteCollectionForAllSites();
+ $requestContext = new RequestContext(
+ '',
+ $request->getMethod(),
+ (string)idn_to_ascii($uri->getHost()),
+ $uri->getScheme(),
+ // Ports are only necessary for URL generation in Symfony which is not used by TYPO3
+ 80,
+ 443,
+ $uri->getPath()
+ );
+ $matcher = new BestUrlMatcher($collection, $requestContext);
+ try {
+ /** @var array{site: SiteInterface, language: ?SiteLanguage, tail: string} $match */
+ $match = $matcher->match($uri->getPath());
+ return new SiteRouteResult(
+ $uri,
+ $match['site'],
+ $match['language'],
+ $match['tail']
+ );
+ } catch (NoConfigurationException | ResourceNotFoundException $e) {
+ return new SiteRouteResult($uri, new NullSite(), null, '');
+ }
+ }
+
+ protected function matchSiteByQueryParams(
+ int $pageId,
+ int $languageId,
+ SiteRouteResult $fallback,
+ UriInterface $uri
+ ): SiteRouteResult {
+ try {
+ $site = $this->finder->getSiteByPageId($pageId);
+ } catch (SiteNotFoundException $e) {
+ return $fallback;
+ }
+
+ try {
+ // override/set language by `&L=` query param
+ $language = $site->getLanguageById($languageId);
+ } catch (\InvalidArgumentException $e) {
+ return $fallback;
+ }
+
+ return new SiteRouteResult($uri, $site, $language);
+ }
}
diff --git a/typo3/sysext/core/Classes/Routing/SiteRouteResult.php b/typo3/sysext/core/Classes/Routing/SiteRouteResult.php
index eba5b416367d..ccb0188072fe 100644
--- a/typo3/sysext/core/Classes/Routing/SiteRouteResult.php
+++ b/typo3/sysext/core/Classes/Routing/SiteRouteResult.php
@@ -95,6 +95,17 @@ class SiteRouteResult implements RouteResultInterface
return in_array($offset, $this->validProperties, true) || isset($this->data[$offset]);
}
+ /**
+ * @internal
+ */
+ public function withLanguage(SiteLanguage $language): self
+ {
+ $clone = clone $this;
+ $clone->language = $language;
+
+ return $clone;
+ }
+
/**
* @param mixed $offset
* @return mixed|UriInterface|string|SiteInterface|SiteLanguage
diff --git a/typo3/sysext/core/Configuration/DefaultConfiguration.php b/typo3/sysext/core/Configuration/DefaultConfiguration.php
index 8e2db8231f3e..46b6f82b235b 100644
--- a/typo3/sysext/core/Configuration/DefaultConfiguration.php
+++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php
@@ -75,6 +75,7 @@ return [
'runtimeDbQuotingOfTcaConfiguration' => true,
'security.frontend.htmlSanitizeParseFuncDefault' => true,
'security.frontend.enforceLoginSigning' => true,
+ 'security.frontend.allowInsecureSiteResolutionByQueryParameters' => false,
'security.backend.htmlSanitizeRte' => false,
'security.backend.enforceReferrer' => true,
'yamlImportsFollowDeclarationOrder' => false,
diff --git a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
index 47ed70cda67b..47c421d2a47a 100644
--- a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
+++ b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
@@ -225,6 +225,9 @@ SYS:
yamlImportsFollowDeclarationOrder:
type: bool
description: 'If on, the YAML imports are imported in the order they are defined in the importing YAML configuration.'
+ security.frontend.allowInsecureSiteResolutionByQueryParameters:
+ type: bool
+ description: 'If on, site resolution can be overwritten by `&id=...&L=...` parameters, URI path & host are just used as default.'
availablePasswordHashAlgorithms:
type: array
description: 'A list of available password hash mechanisms. Extensions may register additional mechanisms here. This is usually not extended in LocalConfiguration.php.'
diff --git a/typo3/sysext/core/Tests/Unit/Routing/SiteMatcherTest.php b/typo3/sysext/core/Tests/Unit/Routing/SiteMatcherTest.php
index 004ff688b547..75f20fb5ea89 100644
--- a/typo3/sysext/core/Tests/Unit/Routing/SiteMatcherTest.php
+++ b/typo3/sysext/core/Tests/Unit/Routing/SiteMatcherTest.php
@@ -17,11 +17,13 @@ declare(strict_types=1);
namespace TYPO3\CMS\Core\Tests\Unit\Routing;
+use TYPO3\CMS\Core\Configuration\Features;
use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\CMS\Core\Routing\SiteMatcher;
use TYPO3\CMS\Core\Routing\SiteRouteResult;
use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Site\SiteFinder;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
class SiteMatcherTest extends UnitTestCase
@@ -71,6 +73,8 @@ class SiteMatcherTest extends UnitTestCase
],
],
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$finderMock = $this->createSiteFinderMock($site, $secondSite);
$subject = new SiteMatcher($finderMock);
@@ -167,6 +171,8 @@ class SiteMatcherTest extends UnitTestCase
],
],
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$finderMock = $this->createSiteFinderMock($site, $secondSite);
$subject = new SiteMatcher($finderMock);
@@ -248,6 +254,8 @@ class SiteMatcherTest extends UnitTestCase
],
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$finderMock = $this->createSiteFinderMock($mainSite, $dkSite, $frSite);
$subject = new SiteMatcher($finderMock);
@@ -259,6 +267,18 @@ class SiteMatcherTest extends UnitTestCase
self::assertSame($expectedLocale, $result->getLanguage()->getLocale());
}
+ private function createFeaturesMock(): Features
+ {
+ $mock = $this->getMockBuilder(Features::class)
+ ->onlyMethods(['isFeatureEnabled'])
+ ->getMock();
+ $mock->expects(self::any())
+ ->method('isFeatureEnabled')
+ ->with('security.frontend.allowInsecureSiteResolutionByQueryParameters')
+ ->willReturn(false);
+ return $mock;
+ }
+
private function createSiteFinderMock(Site ...$sites): SiteFinder
{
$finderMock = $this
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest.php
new file mode 100644
index 000000000000..1831ec2ae1d6
--- /dev/null
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest.php
@@ -0,0 +1,116 @@
+<?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\Frontend\Tests\Functional\SiteHandling;
+
+use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerFactory;
+use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerWriter;
+use TYPO3\TestingFramework\Core\Functional\Framework\Frontend\InternalRequest;
+
+final class SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest extends AbstractTestCase
+{
+ protected $configurationToUseInTestInstance = [
+ 'SYS' => [
+ 'devIPmask' => '123.123.123.123',
+ 'encryptionKey' => '4408d27a916d51e624b69af3554f516dbab61037a9f7b9fd6f81b4d3bedeccb6',
+ 'features' => [
+ 'security.frontend.allowInsecureSiteResolutionByQueryParameters' => false,
+ ],
+ ],
+ 'FE' => [
+ 'cacheHash' => [
+ 'requireCacheHashPresenceParameters' => ['value', 'testing[value]', 'tx_testing_link[value]'],
+ 'excludedParameters' => ['L', 'tx_testing_link[excludedValue]'],
+ 'enforceValidation' => true,
+ ],
+ 'debug' => false,
+ ],
+ ];
+
+ protected function setUp(): void
+ {
+ parent::setUp();
+ $this->withDatabaseSnapshot(function () {
+ $this->importCSVDataSet(__DIR__ . '/../Fixtures/be_users.csv');
+ $backendUser = $this->setUpBackendUser(1);
+ Bootstrap::initializeLanguageObject();
+ $scenarioFile = __DIR__ . '/Fixtures/SlugScenario.yaml';
+ $factory = DataHandlerFactory::fromYamlFile($scenarioFile);
+ $writer = DataHandlerWriter::withBackendUser($backendUser);
+ $writer->invokeFactory($factory);
+ static::failIfArrayIsNotEmpty($writer->getErrors());
+ $this->setUpFrontendRootPage(
+ 1000,
+ [
+ 'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+ 'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+ ],
+ [
+ 'title' => 'ACME Root',
+ ]
+ );
+ $this->setUpFrontendRootPage(
+ 3000,
+ [
+ 'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+ 'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+ ],
+ [
+ 'title' => 'ACME Archive',
+ ]
+ );
+ });
+ }
+
+ public static function siteWithPageIdRequestsAreCorrectlyHandledDataProvider(): \Generator
+ {
+ yield 'valid same-site request is redirected' => ['https://website.local/?id=1000&L=0', 307];
+ yield 'valid same-site request is processed' => ['https://website.local/?id=1100&L=0', 200];
+ yield 'invalid off-site request with unknown domain is denied' => ['https://otherdomain.website.local/?id=3000&L=0', 404];
+ yield 'invalid off-site request with unknown domain and without L parameter is denied' => ['https://otherdomain.website.local/?id=3000', 404];
+ yield 'invalid cross-site request without L parameter is denied' => ['https://website.local/?id=3000', 404];
+ yield 'invalid cross-site request *not* denied' => ['https://website.local/?id=3000&L=0', 404];
+ }
+
+ /**
+ * @test
+ * @dataProvider siteWithPageIdRequestsAreCorrectlyHandledDataProvider
+ */
+ public function siteWithPageIdRequestsAreCorrectlyHandled(string $uri, int $expectation): void
+ {
+ $this->writeSiteConfiguration(
+ 'website-local',
+ $this->buildSiteConfiguration(1000, 'https://website.local/'),
+ [
+ $this->buildDefaultLanguageConfiguration('EN', '/'),
+ ],
+ $this->buildErrorHandlingConfiguration('Fluid', [404])
+ );
+ $this->writeSiteConfiguration(
+ 'archive-acme-com',
+ $this->buildSiteConfiguration(3000, 'https://archive.acme.com/'),
+ [
+ $this->buildDefaultLanguageConfiguration('EN', '/'),
+ ],
+ $this->buildErrorHandlingConfiguration('Fluid', [404])
+ );
+
+ $response = $this->executeFrontendSubRequest(new InternalRequest($uri));
+ self::assertSame($expectation, $response->getStatusCode());
+ }
+}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest.php
new file mode 100644
index 000000000000..39ce557dfbce
--- /dev/null
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest.php
@@ -0,0 +1,118 @@
+<?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\Frontend\Tests\Functional\SiteHandling;
+
+use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerFactory;
+use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerWriter;
+use TYPO3\TestingFramework\Core\Functional\Framework\Frontend\InternalRequest;
+
+final class SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest extends AbstractTestCase
+{
+ protected $configurationToUseInTestInstance = [
+ 'SYS' => [
+ 'devIPmask' => '123.123.123.123',
+ 'encryptionKey' => '4408d27a916d51e624b69af3554f516dbab61037a9f7b9fd6f81b4d3bedeccb6',
+ 'features' => [
+ 'security.frontend.allowInsecureSiteResolutionByQueryParameters' => true,
+ ],
+ ],
+ 'FE' => [
+ 'cacheHash' => [
+ 'requireCacheHashPresenceParameters' => ['value', 'testing[value]', 'tx_testing_link[value]'],
+ 'excludedParameters' => ['L', 'tx_testing_link[excludedValue]'],
+ 'enforceValidation' => true,
+ ],
+ 'debug' => false,
+ ],
+ ];
+
+ protected function setUp(): void
+ {
+ parent::setUp();
+ $this->withDatabaseSnapshot(function () {
+ $this->importCSVDataSet(__DIR__ . '/../Fixtures/be_users.csv');
+ $backendUser = $this->setUpBackendUser(1);
+ Bootstrap::initializeLanguageObject();
+ $scenarioFile = __DIR__ . '/Fixtures/SlugScenario.yaml';
+ $factory = DataHandlerFactory::fromYamlFile($scenarioFile);
+ $writer = DataHandlerWriter::withBackendUser($backendUser);
+ $writer->invokeFactory($factory);
+ static::failIfArrayIsNotEmpty($writer->getErrors());
+ $this->setUpFrontendRootPage(
+ 1000,
+ [
+ 'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+ 'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+ ],
+ [
+ 'title' => 'ACME Root',
+ ]
+ );
+ $this->setUpFrontendRootPage(
+ 3000,
+ [
+ 'typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.typoscript',
+ 'typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/JsonRenderer.typoscript',
+ ],
+ [
+ 'title' => 'ACME Archive',
+ ]
+ );
+ });
+ }
+
+ public static function siteWithPageIdRequestsAreCorrectlyHandledDataProvider(): \Generator
+ {
+ yield 'valid same-site request is redirected' => ['https://website.local/?id=1000&L=0', 307];
+ yield 'valid same-site request is processed' => ['https://website.local/?id=1100&L=0', 200];
+ // This case is allowed due to security.frontend.allowInsecureSiteResolutionByQueryParameters, should otherwise be 404
+ yield 'invalid off-site request with unknown domain is denied' => ['https://otherdomain.website.local/?id=3000&L=0', 200];
+ yield 'invalid off-site request with unknown domain and without L parameter is denied' => ['https://otherdomain.website.local/?id=3000', 404];
+ yield 'invalid cross-site request without L parameter is denied' => ['https://website.local/?id=3000', 404];
+ // This case is allowed due to security.frontend.allowInsecureSiteResolutionByQueryParameters, should otherwise be 404
+ yield 'invalid cross-site request *not* denied' => ['https://website.local/?id=3000&L=0', 200];
+ }
+
+ /**
+ * @test
+ * @dataProvider siteWithPageIdRequestsAreCorrectlyHandledDataProvider
+ */
+ public function siteWithPageIdRequestsAreCorrectlyHandled(string $uri, int $expectation): void
+ {
+ $this->writeSiteConfiguration(
+ 'website-local',
+ $this->buildSiteConfiguration(1000, 'https://website.local/'),
+ [
+ $this->buildDefaultLanguageConfiguration('EN', '/'),
+ ],
+ $this->buildErrorHandlingConfiguration('Fluid', [404])
+ );
+ $this->writeSiteConfiguration(
+ 'archive-acme-com',
+ $this->buildSiteConfiguration(3000, 'https://archive.acme.com/'),
+ [
+ $this->buildDefaultLanguageConfiguration('EN', '/'),
+ ],
+ $this->buildErrorHandlingConfiguration('Fluid', [404])
+ );
+
+ $response = $this->executeFrontendSubRequest(new InternalRequest($uri));
+ self::assertSame($expectation, $response->getStatusCode());
+ }
+}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
index 474f1eb31965..d33b27c775da 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
@@ -316,9 +316,15 @@ class SlugSiteRequestTest extends AbstractTestCase
{
yield 'valid same-site request is redirected' => ['https://website.local/?id=1000&L=0', 307];
yield 'valid same-site request is processed' => ['https://website.local/?id=1100&L=0', 200];
+ yield 'invalid off-site request with unknown domain is denied' => ['https://otherdomain.website.local/?id=3000&L=0', 404];
+ yield 'invalid off-site request with unknown domain and without L parameter is denied' => ['https://otherdomain.website.local/?id=3000', 404];
}
/**
+ * For variants, please see `SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersEnabledTest`
+ * and `SlugSiteRequestAllowInsecureSiteResolutionByQueryParametersDisabledTest` which had to be placed
+ * in separate test class files, due to hard limitations of the TYPO3 Testing Framework.
+ *
* @test
* @dataProvider siteWithPageIdRequestsAreCorrectlyHandledDataProvider
*/
diff --git a/typo3/sysext/frontend/Tests/Unit/Middleware/SiteResolverTest.php b/typo3/sysext/frontend/Tests/Unit/Middleware/SiteResolverTest.php
index 269d6f72f7f1..044c69fbdbb4 100644
--- a/typo3/sysext/frontend/Tests/Unit/Middleware/SiteResolverTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/Middleware/SiteResolverTest.php
@@ -22,6 +22,7 @@ use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;
use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Configuration\Features;
use TYPO3\CMS\Core\Http\JsonResponse;
use TYPO3\CMS\Core\Http\NullResponse;
use TYPO3\CMS\Core\Http\ServerRequest;
@@ -120,6 +121,8 @@ class SiteResolverTest extends UnitTestCase
]),
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$subject = new SiteResolver(new SiteMatcher($this->siteFinder));
$request = new ServerRequest($incomingUrl, 'GET');
@@ -173,6 +176,8 @@ class SiteResolverTest extends UnitTestCase
]),
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$subject = new SiteResolver(new SiteMatcher($this->siteFinder));
$request = new ServerRequest($incomingUrl, 'GET');
@@ -263,6 +268,8 @@ class SiteResolverTest extends UnitTestCase
]),
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$subject = new SiteResolver(new SiteMatcher($this->siteFinder));
$request = new ServerRequest($incomingUrl, 'GET');
@@ -373,6 +380,8 @@ class SiteResolverTest extends UnitTestCase
]),
]);
+ $featuresMock = $this->createFeaturesMock();
+ GeneralUtility::addInstance(Features::class, $featuresMock);
$subject = new SiteResolver(new SiteMatcher($this->siteFinder));
$request = new ServerRequest($incomingUrl, 'GET');
@@ -389,4 +398,16 @@ class SiteResolverTest extends UnitTestCase
self::assertEquals($expectedBase, $result['language-base']);
}
}
+
+ private function createFeaturesMock(): Features
+ {
+ $mock = $this->getMockBuilder(Features::class)
+ ->onlyMethods(['isFeatureEnabled'])
+ ->getMock();
+ $mock->expects(self::any())
+ ->method('isFeatureEnabled')
+ ->with('security.frontend.allowInsecureSiteResolutionByQueryParameters')
+ ->willReturn(false);
+ return $mock;
+ }
}
--
GitLab