From 7006388a47d89ebf47c1ac23d4e408ba2814015d Mon Sep 17 00:00:00 2001
From: Thomas Hohn <thomas@hohn.dk>
Date: Thu, 2 Mar 2023 10:10:27 +0100
Subject: [PATCH] [BUGFIX] Properly escalate if a form HMAC fails to decode
If a HMAC cannot be `json_decode`d (possibly due to old
saved pages or other outdated content), now a proper exception
is thrown instead of issuing a PHP warning.
Resolves: #97337
Releases: main, 12.4, 11.5
Change-Id: I12f5633a85508bf4099d72e474c24b5a0100498c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81719
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benjamin Franzke <ben@bnf.dev>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
---
.../Error/AbstractExceptionHandler.php | 1 +
...MvcPropertyMappingConfigurationService.php | 11 ++++-
...ropertyMappingConfigurationServiceTest.php | 43 +++++++++++++++++++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/typo3/sysext/core/Classes/Error/AbstractExceptionHandler.php b/typo3/sysext/core/Classes/Error/AbstractExceptionHandler.php
index 2dd0aee73903..124f6d2762bb 100644
--- a/typo3/sysext/core/Classes/Error/AbstractExceptionHandler.php
+++ b/typo3/sysext/core/Classes/Error/AbstractExceptionHandler.php
@@ -47,6 +47,7 @@ abstract class AbstractExceptionHandler implements ExceptionHandlerInterface, Si
1616175867, // Backend login request is rate limited
1616175847, // Frontend login request is rate limited
1436717275, // Request with unsupported HTTP method
+ 1699604555, // Outdated __trustedProperties format in extbase property mapping
];
public const IGNORED_HMAC_EXCEPTION_CODES = [
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php b/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
index 2ac1154fbc73..cfaa2a47b398 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
@@ -137,11 +137,18 @@ class MvcPropertyMappingConfigurationService implements SingletonInterface
}
try {
- $serializedTrustedProperties = $this->hashService->validateAndStripHmac($trustedPropertiesToken);
+ $encodedTrustedProperties = $this->hashService->validateAndStripHmac($trustedPropertiesToken);
} catch (InvalidHashException | InvalidArgumentForHashGenerationException $e) {
throw new BadRequestException('The HMAC of the form could not be validated.', 1581862822);
}
- $trustedProperties = json_decode($serializedTrustedProperties, true);
+ $trustedProperties = json_decode($encodedTrustedProperties, true);
+ if (!is_array($trustedProperties)) {
+ if (str_starts_with($encodedTrustedProperties, 'a:')) {
+ throw new BadRequestException('Trusted properties used outdated serialization format instead of json.', 1699604555);
+ }
+ throw new BadRequestException('The HMAC of the form could not be utilized.', 1691267306);
+ }
+
foreach ($trustedProperties as $propertyName => $propertyConfiguration) {
if (!$controllerArguments->hasArgument($propertyName)) {
continue;
diff --git a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
index 53fc33e640ae..2fb794edae30 100644
--- a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
@@ -209,6 +209,49 @@ class MvcPropertyMappingConfigurationServiceTest extends UnitTestCase
$requestHashService->initializePropertyMappingConfigurationFromRequest($request, $arguments);
}
+ /**
+ * @test
+ */
+ public function initializePropertyMappingConfigurationWithNonDecodableTrustedPropertiesThrowsException(): void
+ {
+ $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'bar';
+ $hashService = new HashService();
+ $request = $this->getMockBuilder(Request::class)->onlyMethods(['getInternalArgument'])->disableOriginalConstructor()->getMock();
+ $request->method('getInternalArgument')->with('__trustedProperties')->willReturn('garbage' . $hashService->generateHmac('garbage'));
+ $arguments = new Arguments();
+
+ $arguments = new Arguments();
+ $requestHashService = new MvcPropertyMappingConfigurationService();
+ $requestHashService->injectHashService($hashService);
+
+ $this->expectException(BadRequestException::class);
+ $this->expectExceptionMessage('The HMAC of the form could not be utilized.');
+ $this->expectExceptionCode(1691267306);
+
+ $requestHashService->initializePropertyMappingConfigurationFromRequest($request, $arguments);
+ }
+
+ /**
+ * @test
+ */
+ public function initializePropertyMappingConfigurationWithOutdatedTrustedPropertiesThrowsException(): void
+ {
+ $hashService = new HashService();
+ $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'bar';
+ $request = $this->getMockBuilder(Request::class)->onlyMethods(['getInternalArgument'])->disableOriginalConstructor()->getMock();
+ $request->method('getInternalArgument')->with('__trustedProperties')->willReturn('a:1:{s:3:"foo";s:3:"bar";}' . $hashService->generateHmac('a:1:{s:3:"foo";s:3:"bar";}'));
+
+ $arguments = new Arguments();
+ $requestHashService = new MvcPropertyMappingConfigurationService();
+ $requestHashService->injectHashService($hashService);
+
+ $this->expectException(BadRequestException::class);
+ $this->expectExceptionMessage('Trusted properties used outdated serialization format instead of json.');
+ $this->expectExceptionCode(1699604555);
+
+ $requestHashService->initializePropertyMappingConfigurationFromRequest($request, $arguments);
+ }
+
/**
* @test
*/
--
GitLab