From 916bf8fd876cf2ece3751c9307fc67b0f88cd2b7 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 6 Feb 2024 10:06:44 +0100
Subject: [PATCH] [BUGFIX] Properly resolve GET parameter `id`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This change ensures that `?id=` only
validates against an integer-interpreted
value, disallowing `?id=nonsense23` which
previously resolved to "23"
In case an invalid `?id=` parameter value
is provided throw a page not found error.
Resolves: #103057
Releases: main, 12.4
Change-Id: Iaeb3096944b6872b73b056b690bf4f7fabbe9a6b
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82867
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
---
.../core/Classes/Routing/PageRouter.php | 10 ++--
.../core/Classes/Routing/SiteMatcher.php | 7 +++
.../SiteHandling/SiteRequestTest.php | 50 +++++++++++++++++++
.../SiteHandling/SlugSiteRequestTest.php | 50 +++++++++++++++++++
4 files changed, 114 insertions(+), 3 deletions(-)
diff --git a/typo3/sysext/core/Classes/Routing/PageRouter.php b/typo3/sysext/core/Classes/Routing/PageRouter.php
index 3d61b4180b5a..347e6282054b 100644
--- a/typo3/sysext/core/Classes/Routing/PageRouter.php
+++ b/typo3/sysext/core/Classes/Routing/PageRouter.php
@@ -39,6 +39,7 @@ use TYPO3\CMS\Core\Routing\Enhancer\RoutingEnhancerInterface;
use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Frontend\Page\CacheHashCalculator;
/**
@@ -104,9 +105,12 @@ class PageRouter implements RouterInterface
$candidateProvider = $this->getSlugCandidateProvider($this->context);
// Legacy URIs (?id=12345) takes precedence, no matter if a route is given
- $requestId = (int)($request->getQueryParams()['id'] ?? 0);
- if ($requestId > 0) {
- if (!empty($pageId = $candidateProvider->getRealPageIdForPageIdAsPossibleCandidate($requestId))) {
+ $requestId = ($request->getQueryParams()['id'] ?? null);
+ if ($requestId !== null) {
+ if (MathUtility::canBeInterpretedAsInteger($requestId)
+ && (int)$requestId > 0
+ && !empty($pageId = $candidateProvider->getRealPageIdForPageIdAsPossibleCandidate((int)$requestId))
+ ) {
return new PageArguments(
(int)$pageId,
(string)($request->getQueryParams()['type'] ?? '0'),
diff --git a/typo3/sysext/core/Classes/Routing/SiteMatcher.php b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
index 639080c0d3d6..36ce7e35c5d3 100644
--- a/typo3/sysext/core/Classes/Routing/SiteMatcher.php
+++ b/typo3/sysext/core/Classes/Routing/SiteMatcher.php
@@ -31,6 +31,7 @@ use TYPO3\CMS\Core\Site\Entity\SiteInterface;
use TYPO3\CMS\Core\Site\Entity\SiteLanguage;
use TYPO3\CMS\Core\Site\SiteFinder;
use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Core\Utility\RootlineUtility;
/**
@@ -185,6 +186,9 @@ class SiteMatcher implements SingletonInterface
if ($pageId === null) {
return null;
}
+ if (!MathUtility::canBeInterpretedAsInteger($pageId)) {
+ return null;
+ }
return (int)$pageId <= 0 ? null : (int)$pageId;
}
@@ -197,6 +201,9 @@ class SiteMatcher implements SingletonInterface
if ($languageId === null) {
return null;
}
+ if (!MathUtility::canBeInterpretedAsInteger($languageId)) {
+ return null;
+ }
return (int)$languageId < 0 ? null : (int)$languageId;
}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
index 8d050f2d35ea..84c4ec6a8271 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SiteRequestTest.php
@@ -17,7 +17,10 @@ declare(strict_types=1);
namespace TYPO3\CMS\Frontend\Tests\Functional\SiteHandling;
+use Psr\Http\Message\UriInterface;
use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\CMS\Core\Http\Uri;
+use TYPO3\CMS\Core\Utility\HttpUtility;
use TYPO3\CMS\Core\Utility\PermutationUtility;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerFactory;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerWriter;
@@ -1057,4 +1060,51 @@ final class SiteRequestTest extends AbstractTestCase
self::stringContains('ID was outside the domain')
);
}
+
+ public static function getUrisWithInvalidLegacyQueryParameters(): \Generator
+ {
+ $uri = new Uri('https://website.local/');
+ yield '#0 id with float value having a zero decimal' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '1110.0'])),
+ ];
+ yield '#1 id string value with tailing numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 'step1110'])),
+ ];
+ yield '#2 id string value with leading numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '1110step'])),
+ ];
+ yield '#3 id string value without numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 'foobar'])),
+ ];
+ yield '#4 id string value with a exponent' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '11e10'])),
+ ];
+ yield '#5 id with a zero as value' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 0])),
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider getUrisWithInvalidLegacyQueryParameters
+ */
+ public function requestWithInvalidLegacyQueryParametersDisplayPageNotFoundPage(UriInterface $uri): void
+ {
+ $this->writeSiteConfiguration(
+ 'website-local',
+ $this->buildSiteConfiguration(1000, 'https://website.local/'),
+ [],
+ $this->buildErrorHandlingConfiguration('PHP', [404])
+ );
+ $response = $this->executeFrontendSubRequest(
+ new InternalRequest((string)$uri),
+ new InternalRequestContext()
+ );
+ $json = json_decode((string)$response->getBody(), true);
+ self::assertSame(404, $response->getStatusCode());
+ self::assertThat(
+ $json['message'] ?? null,
+ self::stringContains('The requested page does not exist')
+ );
+ }
}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
index 38a1cb4d2af8..32357b5cf688 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
@@ -17,7 +17,10 @@ declare(strict_types=1);
namespace TYPO3\CMS\Frontend\Tests\Functional\SiteHandling;
+use Psr\Http\Message\UriInterface;
use TYPO3\CMS\Core\Core\Bootstrap;
+use TYPO3\CMS\Core\Http\Uri;
+use TYPO3\CMS\Core\Utility\HttpUtility;
use TYPO3\CMS\Core\Utility\PermutationUtility;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerFactory;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerWriter;
@@ -2767,4 +2770,51 @@ final class SlugSiteRequestTest extends AbstractTestCase
);
}
}
+
+ public static function getUrisWithInvalidLegacyQueryParameters(): \Generator
+ {
+ $uri = new Uri('https://website.local/welcome/');
+ yield '#0 id with float value having a zero decimal' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '1110.0'])),
+ ];
+ yield '#1 id string value with tailing numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 'step1110'])),
+ ];
+ yield '#2 id string value with leading numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '1110step'])),
+ ];
+ yield '#3 id string value without numbers' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 'foobar'])),
+ ];
+ yield '#4 id string value with a exponent' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => '11e10'])),
+ ];
+ yield '#5 id with a zero as value' => [
+ 'uri' => $uri->withQuery(HttpUtility::buildQueryString(['id' => 0])),
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider getUrisWithInvalidLegacyQueryParameters
+ */
+ public function requestWithInvalidLegacyQueryParametersDisplayPageNotFoundPage(UriInterface $uri): void
+ {
+ $this->writeSiteConfiguration(
+ 'website-local',
+ $this->buildSiteConfiguration(1000, 'https://website.local/'),
+ [],
+ $this->buildErrorHandlingConfiguration('PHP', [404])
+ );
+ $response = $this->executeFrontendSubRequest(
+ new InternalRequest((string)$uri),
+ new InternalRequestContext()
+ );
+ $json = json_decode((string)$response->getBody(), true);
+ self::assertSame(404, $response->getStatusCode());
+ self::assertThat(
+ $json['message'] ?? null,
+ self::stringContains('The requested page does not exist')
+ );
+ }
}
--
GitLab