[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>
TYPO3 · Nov 10, 2023 · 7006388 · 7006388
1 parent 1ee881f
commit 7006388
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 2 deletions.
diff --git 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
@@ -137,11 +137,18 @@ public function initializePropertyMappingConfigurationFromRequest(Request $reque
         }
 
         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/...3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php b/...3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
@@ -209,6 +209,49 @@ public function initializePropertyMappingConfigurationThrowsBadRequestExceptionO
         $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
      */