From f553d918cb69eb9dc525cad689ce44c08e9b5f43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20E=C3=9Fl?= <indy.essl@gmail.com>
Date: Sun, 16 Feb 2020 14:38:41 +0100
Subject: [PATCH] [BUGFIX] Throw BadRequestException on failed hmac validation
from forms
If a HMAC of a submitted form is invalid (because it has been tampered
with), TYPO3 would previously throw an exception that leads to a
status code 500. This is incorrect behaviour, as the error comes from
bad user input and not a server error.
In case the HMAC of a submitted form is invalid, both extbase and
ext:form will now throw a BadRequestException, which will then lead to
a status code 400 (BAD REQUEST).
Resolves: #90134
Releases: master, 9.5
Change-Id: If4dad7ba27190b5992bab68b4ce64a423c0db645
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63272
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
---
...MvcPropertyMappingConfigurationService.php | 11 ++++++++++-
...ropertyMappingConfigurationServiceTest.php | 19 +++++++++++++++++++
.../Classes/Domain/Runtime/FormRuntime.php | 10 +++++++++-
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php b/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
index 355d72fafb6a..2dff40ad45ca 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/MvcPropertyMappingConfigurationService.php
@@ -14,6 +14,10 @@ namespace TYPO3\CMS\Extbase\Mvc\Controller;
* The TYPO3 project - inspiring people to share!
*/
+use TYPO3\CMS\Core\Error\Http\BadRequestException;
+use TYPO3\CMS\Extbase\Security\Exception\InvalidArgumentForHashGenerationException;
+use TYPO3\CMS\Extbase\Security\Exception\InvalidHashException;
+
/**
* This is a Service which can generate a request hash and check whether the currently given arguments
* fit to the request hash.
@@ -117,6 +121,7 @@ class MvcPropertyMappingConfigurationService implements \TYPO3\CMS\Core\Singleto
*
* @param \TYPO3\CMS\Extbase\Mvc\Request $request
* @param \TYPO3\CMS\Extbase\Mvc\Controller\Arguments $controllerArguments
+ * @throws BadRequestException
*/
public function initializePropertyMappingConfigurationFromRequest(\TYPO3\CMS\Extbase\Mvc\Request $request, \TYPO3\CMS\Extbase\Mvc\Controller\Arguments $controllerArguments)
{
@@ -125,7 +130,11 @@ class MvcPropertyMappingConfigurationService implements \TYPO3\CMS\Core\Singleto
return;
}
- $serializedTrustedProperties = $this->hashService->validateAndStripHmac($trustedPropertiesToken);
+ try {
+ $serializedTrustedProperties = $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);
foreach ($trustedProperties as $propertyName => $propertyConfiguration) {
if (!$controllerArguments->hasArgument($propertyName)) {
diff --git a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
index 775577a43fe9..bad2986ed18e 100644
--- a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/MvcPropertyMappingConfigurationServiceTest.php
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Extbase\Tests\Unit\Mvc\Controller;
* The TYPO3 project - inspiring people to share!
*/
+use TYPO3\CMS\Core\Error\Http\BadRequestException;
use TYPO3\CMS\Extbase\Mvc\Controller\MvcPropertyMappingConfigurationService;
use TYPO3\CMS\Extbase\Security\Cryptography\HashService;
use TYPO3\CMS\Extbase\Security\Exception\InvalidArgumentForHashGenerationException;
@@ -184,6 +185,24 @@ class MvcPropertyMappingConfigurationServiceTest extends UnitTestCase
$requestHashService->initializePropertyMappingConfigurationFromRequest($request, $arguments);
}
+ /**
+ * @test
+ */
+ public function initializePropertyMappingConfigurationThrowsBadRequestExceptionOnInvalidHmac()
+ {
+ $this->expectException(BadRequestException::class);
+ $this->expectExceptionCode(1581862822);
+
+ $request = $this->getMockBuilder(\TYPO3\CMS\Extbase\Mvc\Request::class)->setMethods(['getInternalArgument'])->disableOriginalConstructor()->getMock();
+ $request->expects(self::any())->method('getInternalArgument')->with('__trustedProperties')->willReturn('string with less than 40 characters');
+ $arguments = new \TYPO3\CMS\Extbase\Mvc\Controller\Arguments();
+
+ $hashService = new \TYPO3\CMS\Extbase\Security\Cryptography\HashService();
+ $requestHashService = new MvcPropertyMappingConfigurationService;
+ $requestHashService->injectHashService($hashService);
+ $requestHashService->initializePropertyMappingConfigurationFromRequest($request, $arguments);
+ }
+
/**
* @test
*/
diff --git a/typo3/sysext/form/Classes/Domain/Runtime/FormRuntime.php b/typo3/sysext/form/Classes/Domain/Runtime/FormRuntime.php
index 252f73d0cd87..3e76003e70ca 100644
--- a/typo3/sysext/form/Classes/Domain/Runtime/FormRuntime.php
+++ b/typo3/sysext/form/Classes/Domain/Runtime/FormRuntime.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Form\Domain\Runtime;
use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\Context\Context;
+use TYPO3\CMS\Core\Error\Http\BadRequestException;
use TYPO3\CMS\Core\ExpressionLanguage\Resolver;
use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\CMS\Core\Site\Entity\Site;
@@ -33,6 +34,8 @@ use TYPO3\CMS\Extbase\Mvc\Web\Request;
use TYPO3\CMS\Extbase\Mvc\Web\Response;
use TYPO3\CMS\Extbase\Mvc\Web\Routing\UriBuilder;
use TYPO3\CMS\Extbase\Property\Exception as PropertyException;
+use TYPO3\CMS\Extbase\Security\Exception\InvalidArgumentForHashGenerationException;
+use TYPO3\CMS\Extbase\Security\Exception\InvalidHashException;
use TYPO3\CMS\Form\Domain\Exception\RenderingException;
use TYPO3\CMS\Form\Domain\Finishers\FinisherContext;
use TYPO3\CMS\Form\Domain\Finishers\FinisherInterface;
@@ -210,6 +213,7 @@ class FormRuntime implements RootRenderableInterface, \ArrayAccess
/**
* Initializes the current state of the form, based on the request
+ * @throws BadRequestException
*/
protected function initializeFormStateFromRequest()
{
@@ -217,7 +221,11 @@ class FormRuntime implements RootRenderableInterface, \ArrayAccess
if ($serializedFormStateWithHmac === null) {
$this->formState = GeneralUtility::makeInstance(FormState::class);
} else {
- $serializedFormState = $this->hashService->validateAndStripHmac($serializedFormStateWithHmac);
+ try {
+ $serializedFormState = $this->hashService->validateAndStripHmac($serializedFormStateWithHmac);
+ } catch (InvalidHashException | InvalidArgumentForHashGenerationException $e) {
+ throw new BadRequestException('The HMAC of the form could not be validated.', 1581862823);
+ }
$this->formState = unserialize(base64_decode($serializedFormState));
}
}
--
GitLab