From 471a83bc74270269e64d5e44096f2617a59f7076 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Thu, 14 Mar 2024 18:36:27 +0100
Subject: [PATCH] [BUGFIX] Avoid mapping route values that are out of scope
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Given a static route argument mapper is used - for instance one
that allows values in the range of 1 to 100 - then generating an
URL from a route with an out of scope `&value=5000` was still
generating an URL. However, HTTP request to that URL would
result in an 404 error.
This change skips routes using out of scope values during the
URL generation process to avoid pointing to invalid resources.
Resolves: #103400
Releases: main, 12.4, 11.5
Change-Id: I11e12f29e294ec86bec948d3a922d4a56b231771
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83470
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Klee <typo3-coding@oliverklee.de>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../core/Classes/Routing/PageRouter.php | 5 +-
.../EnhancerLinkGenerator/RouteTest.php | 65 +++++++++++++++++++
.../Framework/Builder/VariableValue.php | 49 ++++++++++++--
3 files changed, 113 insertions(+), 6 deletions(-)
diff --git a/typo3/sysext/core/Classes/Routing/PageRouter.php b/typo3/sysext/core/Classes/Routing/PageRouter.php
index 18ec33dbc192..d80ce5d06139 100644
--- a/typo3/sysext/core/Classes/Routing/PageRouter.php
+++ b/typo3/sysext/core/Classes/Routing/PageRouter.php
@@ -346,7 +346,10 @@ class PageRouter implements RouterInterface
if ($route->hasOption('deflatedParameters')) {
$parameters = $route->getOption('deflatedParameters');
}
- $mappableProcessor->generate($route, $parameters);
+ // skip the route, in case any aspect of it could not be mapped to a value
+ if ($mappableProcessor->generate($route, $parameters) === false) {
+ continue;
+ }
// ABSOLUTE_URL is used as default fallback
$urlAsString = $generator->generate($routeName, $parameters, $referenceType);
$uri = new Uri($urlAsString);
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGenerator/RouteTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGenerator/RouteTest.php
index dc1c080f1888..504d67050cfe 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGenerator/RouteTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGenerator/RouteTest.php
@@ -453,4 +453,69 @@ final class RouteTest extends AbstractEnhancerLinkGeneratorTestCase
{
$this->assertGeneratedUriEquals($testSet);
}
+
+ public static function outOfScopeValueIsNotMappedDataProvider(): array
+ {
+ // variables (applied when invoking expectations)
+ $variables = Variables::create()->define([
+ 'value' => null, // defined via VariableContext
+ 'routePrefix' => 'enhance',
+ 'aspectName' => 'value',
+ ]);
+ $variableContexts = [];
+ $enhancers = Builder::create()->declareEnhancers();
+ foreach ($enhancers as $enhancer) {
+ $generatedParams = match ($enhancer->describe()) {
+ 'Simple' => '&value=[[value]]',
+ 'Plugin' => '&testing[value]=[[value]]',
+ 'Extbase' => '&tx_testing_link[action]=index&tx_testing_link[controller]=Link&tx_testing_link[value]=[[value]]',
+ default => null,
+ };
+ if ($generatedParams === null) {
+ continue;
+ }
+ $variableContexts[] = VariablesContext::create(
+ Variables::create([
+ 'generatedParams' => VariableValue::createUrlEncodedParams($generatedParams, null, '?'),
+ ])
+ )->withRequiredApplicables($enhancer);
+ }
+ return Permutation::create($variables)
+ ->withTargets(
+ TestSet::create()
+ ->withMergedApplicables(LanguageContext::create(0))
+ ->withTargetPageId(1100)
+ ->withUrl(
+ VariableValue::create('https://acme.us/welcome[[generatedParams]]&cHash=')
+ ),
+ )
+ ->withApplicableSet(
+ // value `5000` is out of scope for the given range `[1; 100]`
+ VariablesContext::create(Variables::create(['value' => 5000])),
+ )
+ ->withApplicableItems($enhancers)
+ ->withApplicableItems($variableContexts)
+ ->withApplicableSet(
+ AspectDeclaration::create('StaticRangeMapper')->withConfiguration([
+ VariableItem::create('aspectName', [
+ 'type' => 'StaticRangeMapper',
+ 'start' => '1',
+ 'end' => '100',
+ ]),
+ ])
+ )
+ ->permute()
+ ->getTargetsForDataProvider();
+ }
+
+ /**
+ * Asserts that value `5000` cannot generate a valid URL, having
+ * a `StaticRangeMapper` which only allows values in range `[1; 100]`.
+ */
+ #[DataProvider('outOfScopeValueIsNotMappedDataProvider')]
+ #[Test]
+ public function outOfScopeValueIsNotMapped(TestSet $testSet): void
+ {
+ $this->assertGeneratedUriEquals($testSet, false);
+ }
}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/VariableValue.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/VariableValue.php
index 8298de237450..843204e2e092 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/VariableValue.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/VariableValue.php
@@ -40,10 +40,19 @@ class VariableValue
return new static($value, $defaultVariables);
}
+ public static function createUrlEncodedParams(
+ string $value,
+ Variables $defaultVariables = null,
+ string $prefix = '&'
+ ): self {
+ $value = self::urlEncodeParams($value, $prefix);
+ return self::create($value, $defaultVariables);
+ }
+
private function __construct(string $value, Variables $defaultVariables = null)
{
- $variableNames = $this->extractVariableNames($value);
- if (count($variableNames) === 0) {
+ $variableNames = self::extractVariableNames($value);
+ if ($variableNames === []) {
throw new \LogicException(
sprintf(
'Payload did not contain any variables "%s"',
@@ -58,9 +67,15 @@ class VariableValue
$this->defaultVariables = $defaultVariables;
}
+ public function __toString(): string
+ {
+ return $this->value;
+ }
+
public function apply(Variables $variables): string
{
$variables = $variables->withDefined($this->defaultVariables);
+ $variables = $this->resolveNestedVariables($variables);
$this->assertVariableNames($variables);
if (!$this->hasAllRequiredDefinedVariableNames($variables)) {
@@ -68,12 +83,25 @@ class VariableValue
}
return str_replace(
- array_map([$this, 'wrap'], $variables->keys()),
+ array_map([self::class, 'wrap'], $variables->keys()),
$variables->values(),
$this->value
);
}
+ private function resolveNestedVariables(Variables $variables): Variables
+ {
+ $target = clone $variables;
+ foreach ($variables as $key => $value) {
+ if ($value instanceof self) {
+ $otherVariables = clone $variables;
+ unset($otherVariables[$key]);
+ $target[$key] = $value->apply($otherVariables);
+ }
+ }
+ return $target;
+ }
+
private function assertVariableNames(Variables $variables): void
{
$missingVariableNames = array_diff($this->variableNames, $variables->keys());
@@ -89,7 +117,7 @@ class VariableValue
}
}
- private function extractVariableNames(string $value): array
+ private static function extractVariableNames(string $value): array
{
if (!preg_match_all('#\[\[(?P<variableName>[^\[\]]+)\]\]#', $value, $matches)) {
return [];
@@ -97,8 +125,19 @@ class VariableValue
return array_unique($matches['variableName']);
}
- private function wrap(string $item): string
+ private static function wrap(string $item): string
{
return '[[' . $item . ']]';
}
+
+ private static function urlEncodeParams(string $value, string $prefix = '&'): string
+ {
+ $variableNames = self::extractVariableNames($value);
+ $variableItems = array_map([self::class, 'wrap'], $variableNames);
+ $substitutes = array_map(static fn(): string => bin2hex(random_bytes(20)), $variableNames);
+ $value = str_replace($variableItems, $substitutes, $value);
+ parse_str($value, $params);
+ $value = http_build_query($params, '', '&', PHP_QUERY_RFC3986);
+ return $prefix . str_replace($substitutes, $variableItems, $value);
+ }
}
--
GitLab