From 70305f88c5652ef2e5c36cbd1ab0a9a8cd8ab92c Mon Sep 17 00:00:00 2001
From: Torben Hansen <derhansen@gmail.com>
Date: Wed, 6 Mar 2024 18:07:31 +0100
Subject: [PATCH] [TASK] Replace GeneralUtility::hmac usage in ext:form
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
With #102761, the new `HashService` has been introduced to ensure
that HMAC generation will always use an additional secret. As a
follow-up patch, `GeneralUtility::hmac` will be deprecated when all
usages in the core have been replaced with the new `HashService`.
With this change, usages of `GeneralUtility::hmac` in ext:form
are replaced by the new HashService.
Resolves: #103249
Related: #102761
Related: #103245
Releases: main
Change-Id: I636d224764890e31ef33038eb7ec2dd0472786c1
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83266
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
...HmacDataToFormElementPropertyConverter.php | 4 ++-
...taToPropertyCollectionElementConverter.php | 4 ++-
.../FormDefinitionValidationService.php | 7 +++--
.../form/Classes/Hooks/ImportExportHook.php | 6 ++--
.../UploadedFileReferenceConverter.php | 2 +-
.../form/Classes/Slot/FilePersistenceSlot.php | 6 ++--
.../Classes/Slot/ResourcePublicationSlot.php | 5 +++-
.../FormDefinitionValidationServiceTest.php | 28 +++++++++++--------
.../FormDefinitionArrayConverterTest.php | 16 ++++++-----
9 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToFormElementPropertyConverter.php b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToFormElementPropertyConverter.php
index 4f7ee9a7b5ba..ee659081ba2c 100644
--- a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToFormElementPropertyConverter.php
+++ b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToFormElementPropertyConverter.php
@@ -17,6 +17,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Domain\Configuration\FormDefinition\Converters;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\Utility\ArrayUtility;
use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -36,10 +37,11 @@ class AddHmacDataToFormElementPropertyConverter extends AbstractConverter
$lastKeySegment = array_pop($propertyPathParts);
$propertyPathParts[] = '_orig_' . $lastKeySegment;
+ $hashService = GeneralUtility::makeInstance(HashService::class);
$hmacValuePath = implode('.', array_merge($this->converterDto->getRenderablePathParts(), $propertyPathParts));
$hmacValue = [
'value' => $value,
- 'hmac' => GeneralUtility::hmac(serialize([$this->converterDto->getFormElementIdentifier(), $key, $value]), $this->sessionToken),
+ 'hmac' => $hashService->hmac(serialize([$this->converterDto->getFormElementIdentifier(), $key, $value]), $this->sessionToken),
];
$formDefinition = ArrayUtility::setValueByPath($formDefinition, $hmacValuePath, $hmacValue, '.');
diff --git a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToPropertyCollectionElementConverter.php b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToPropertyCollectionElementConverter.php
index e065b832ffd8..d1870ad0339f 100644
--- a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToPropertyCollectionElementConverter.php
+++ b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinition/Converters/AddHmacDataToPropertyCollectionElementConverter.php
@@ -17,6 +17,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Domain\Configuration\FormDefinition\Converters;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\Utility\ArrayUtility;
use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -42,9 +43,10 @@ class AddHmacDataToPropertyCollectionElementConverter extends AbstractConverter
$propertyPathParts
));
+ $hashService = GeneralUtility::makeInstance(HashService::class);
$hmacValue = [
'value' => $value,
- 'hmac' => GeneralUtility::hmac(
+ 'hmac' => $hashService->hmac(
serialize([
$this->converterDto->getFormElementIdentifier(),
$this->converterDto->getPropertyCollectionName(),
diff --git a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinitionValidationService.php b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinitionValidationService.php
index 648a97c7c379..5ab33a36ff3f 100644
--- a/typo3/sysext/form/Classes/Domain/Configuration/FormDefinitionValidationService.php
+++ b/typo3/sysext/form/Classes/Domain/Configuration/FormDefinitionValidationService.php
@@ -17,6 +17,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Domain\Configuration;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Form\Domain\Configuration\ArrayProcessing\ArrayProcessing;
@@ -223,7 +224,8 @@ class FormDefinitionValidationService implements SingletonInterface
$this->checkHmacDataIntegrity($hmacData, $hmacContent, $sessionToken);
$hmacContent[] = $propertyValue;
- $expectedHash = GeneralUtility::hmac(serialize($hmacContent), $sessionToken);
+ $hashService = GeneralUtility::makeInstance(HashService::class);
+ $expectedHash = $hashService->hmac(serialize($hmacContent), $sessionToken);
return hash_equals($expectedHash, $hmacData['hmac']);
}
@@ -242,7 +244,8 @@ class FormDefinitionValidationService implements SingletonInterface
}
$hmacContent[] = $hmacData['value'] ?? '';
- $expectedHash = GeneralUtility::hmac(serialize($hmacContent), $sessionToken);
+ $hashService = GeneralUtility::makeInstance(HashService::class);
+ $expectedHash = $hashService->hmac(serialize($hmacContent), $sessionToken);
if (!hash_equals($expectedHash, $hmac)) {
throw new PropertyException('Unauthorized modification of historical data. #1528538252', 1528538252);
diff --git a/typo3/sysext/form/Classes/Hooks/ImportExportHook.php b/typo3/sysext/form/Classes/Hooks/ImportExportHook.php
index edada2f4cef7..199f09545794 100644
--- a/typo3/sysext/form/Classes/Hooks/ImportExportHook.php
+++ b/typo3/sysext/form/Classes/Hooks/ImportExportHook.php
@@ -28,11 +28,11 @@ class ImportExportHook
$fileRecord = $params['fileRecord'];
$temporaryFile = $params['temporaryFile'];
- $formPersistenceSlot = GeneralUtility::makeInstance(FilePersistenceSlot::class);
- $formPersistenceSlot->allowInvocation(
+ $filePersistenceSlot = GeneralUtility::makeInstance(FilePersistenceSlot::class);
+ $filePersistenceSlot->allowInvocation(
FilePersistenceSlot::COMMAND_FILE_ADD,
implode(':', [$fileRecord['storage'], $fileRecord['identifier']]),
- $formPersistenceSlot->getContentSignature(file_get_contents($temporaryFile))
+ $filePersistenceSlot->getContentSignature(file_get_contents($temporaryFile))
);
}
}
diff --git a/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php b/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
index 366d8891f2e6..5040592a1936 100644
--- a/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
+++ b/typo3/sysext/form/Classes/Mvc/Property/TypeConverter/UploadedFileReferenceConverter.php
@@ -221,7 +221,7 @@ class UploadedFileReferenceConverter extends AbstractTypeConverter
$uploadFolder = $this->provideUploadFolder($uploadFolderId);
// current folder name, derived from public random seed (`formSession`)
- $currentName = 'form_' . GeneralUtility::hmac($seed, self::class);
+ $currentName = 'form_' . $this->hashService->hmac($seed, self::class);
$uploadFolder = $this->provideTargetFolder($uploadFolder, $currentName);
// sub-folder in $uploadFolder with 160 bit of derived entropy (.../form_<40-chars-hash>/actual.file)
$uploadedFile = $uploadFolder->addUploadedFile($uploadInfo, $conflictMode);
diff --git a/typo3/sysext/form/Classes/Slot/FilePersistenceSlot.php b/typo3/sysext/form/Classes/Slot/FilePersistenceSlot.php
index a655dc434034..2c454ca7a2d2 100644
--- a/typo3/sysext/form/Classes/Slot/FilePersistenceSlot.php
+++ b/typo3/sysext/form/Classes/Slot/FilePersistenceSlot.php
@@ -18,6 +18,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Slot;
use TYPO3\CMS\Core\Attribute\AsEventListener;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\Resource\Event\BeforeFileAddedEvent;
use TYPO3\CMS\Core\Resource\Event\BeforeFileContentsSetEvent;
use TYPO3\CMS\Core\Resource\Event\BeforeFileCreatedEvent;
@@ -26,7 +27,6 @@ use TYPO3\CMS\Core\Resource\Event\BeforeFileRenamedEvent;
use TYPO3\CMS\Core\Resource\Event\BeforeFileReplacedEvent;
use TYPO3\CMS\Core\Resource\FolderInterface;
use TYPO3\CMS\Core\SingletonInterface;
-use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Form\Mvc\Persistence\FormPersistenceManager;
/**
@@ -53,9 +53,11 @@ final class FilePersistenceSlot implements SingletonInterface
*/
protected $allowedInvocations = [];
+ public function __construct(private readonly HashService $hashService) {}
+
public function getContentSignature(string $content): string
{
- return GeneralUtility::hmac($content);
+ return $this->hashService->hmac($content, self::class);
}
/**
diff --git a/typo3/sysext/form/Classes/Slot/ResourcePublicationSlot.php b/typo3/sysext/form/Classes/Slot/ResourcePublicationSlot.php
index 9dd9413b3f3e..1200f8e4710f 100644
--- a/typo3/sysext/form/Classes/Slot/ResourcePublicationSlot.php
+++ b/typo3/sysext/form/Classes/Slot/ResourcePublicationSlot.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Form\Slot;
use TYPO3\CMS\Core\Attribute\AsEventListener;
use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\Resource\Event\GeneratePublicUrlForResourceEvent;
use TYPO3\CMS\Core\Resource\File;
use TYPO3\CMS\Core\Resource\FileInterface;
@@ -40,6 +41,8 @@ class ResourcePublicationSlot implements SingletonInterface
*/
protected $fileIdentifiers = [];
+ public function __construct(private readonly HashService $hashService) {}
+
#[AsEventListener('form-framework/resource-getPublicUrl')]
public function getPublicUrl(GeneratePublicUrlForResourceEvent $event): void
{
@@ -77,7 +80,7 @@ class ResourcePublicationSlot implements SingletonInterface
$queryParameterArray['t'] = 'p';
}
- $queryParameterArray['token'] = GeneralUtility::hmac(implode('|', $queryParameterArray), 'resourceStorageDumpFile');
+ $queryParameterArray['token'] = $this->hashService->hmac(implode('|', $queryParameterArray), 'resourceStorageDumpFile');
$publicUrl = GeneralUtility::locationHeaderUrl(PathUtility::getAbsoluteWebPath(Environment::getPublicPath() . '/index.php'));
$publicUrl .= '?' . http_build_query($queryParameterArray, '', '&', PHP_QUERY_RFC3986);
return $publicUrl;
diff --git a/typo3/sysext/form/Tests/Unit/Domain/Configuration/FormDefinitionValidationServiceTest.php b/typo3/sysext/form/Tests/Unit/Domain/Configuration/FormDefinitionValidationServiceTest.php
index 00d5cb672dc6..b17a5024508b 100644
--- a/typo3/sysext/form/Tests/Unit/Domain/Configuration/FormDefinitionValidationServiceTest.php
+++ b/typo3/sysext/form/Tests/Unit/Domain/Configuration/FormDefinitionValidationServiceTest.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Form\Tests\Unit\Domain\Configuration;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Form\Domain\Configuration\ConfigurationService;
use TYPO3\CMS\Form\Domain\Configuration\Exception\PropertyException;
@@ -28,10 +29,13 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
final class FormDefinitionValidationServiceTest extends UnitTestCase
{
+ protected HashService $hashService;
+
protected function setUp(): void
{
parent::setUp();
$GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = '12345';
+ $this->hashService = new HashService();
}
public function tearDown(): void
@@ -59,7 +63,7 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'label' => 'xxx',
'_orig_label' => [
'value' => 'aaa',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'label', 'aaa']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'label', 'aaa']), $sessionToken),
],
];
@@ -104,7 +108,7 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'label' => 'aaa',
'_orig_label' => [
'value' => 'aaa',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'label', 'aaa']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'label', 'aaa']), $sessionToken),
],
];
@@ -141,13 +145,13 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'identifier' => 'StringLength',
'_orig_identifier' => [
'value' => 'StringLength',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
],
'options' => [
'test' => 'xxx',
'_orig_test' => [
'value' => 'aaa',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'options.test', 'aaa']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'options.test', 'aaa']), $sessionToken),
],
],
];
@@ -179,7 +183,7 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'identifier' => 'StringLength',
'_orig_identifier' => [
'value' => 'StringLength',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
],
'options' => [
'test' => 'xxx',
@@ -210,13 +214,13 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'identifier' => 'StringLength',
'_orig_identifier' => [
'value' => 'StringLength',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'identifier', 'StringLength']), $sessionToken),
],
'options' => [
'test' => 'aaa',
'_orig_test' => [
'value' => 'aaa',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'options.test', 'aaa']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'options.test', 'aaa']), $sessionToken),
],
],
];
@@ -244,12 +248,13 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
$sessionToken = '54321';
$identifier = 'text-1';
+ $hashService = new HashService();
$validationDto = new ValidationDto('standard', 'Text', $identifier);
$formElement = [
'test' => 'xxx',
'_orig_test' => [
'value' => 'xxx',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'test', 'xxx']), $sessionToken),
+ 'hmac' => $hashService->hmac(serialize([$identifier, 'test', 'xxx']), $sessionToken),
],
];
@@ -261,7 +266,7 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'test' => 'xxx1',
'_orig_test' => [
'value' => 'xxx',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'test', 'xxx']), $sessionToken),
+ 'hmac' => $hashService->hmac(serialize([$identifier, 'test', 'xxx']), $sessionToken),
],
];
@@ -420,11 +425,12 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
$identifier = 'text-1';
$validationDto = new ValidationDto('standard', 'Text', $identifier, null, 'validators', 'StringLength');
+ $hashService = new HashService();
$formElement = [
'test' => 'xxx',
'_orig_test' => [
'value' => 'xxx',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'test', 'xxx']), $sessionToken),
+ 'hmac' => $hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'test', 'xxx']), $sessionToken),
],
];
@@ -436,7 +442,7 @@ final class FormDefinitionValidationServiceTest extends UnitTestCase
'test' => 'xxx1',
'_orig_test' => [
'value' => 'xxx',
- 'hmac' => GeneralUtility::hmac(serialize([$identifier, 'validators', 'StringLength', 'test', 'xxx']), $sessionToken),
+ 'hmac' => $hashService->hmac(serialize([$identifier, 'validators', 'StringLength', 'test', 'xxx']), $sessionToken),
],
];
diff --git a/typo3/sysext/form/Tests/Unit/Mvc/Property/TypeConverter/FormDefinitionArrayConverterTest.php b/typo3/sysext/form/Tests/Unit/Mvc/Property/TypeConverter/FormDefinitionArrayConverterTest.php
index 3d4b24d0e892..8daeaf7b0ecb 100644
--- a/typo3/sysext/form/Tests/Unit/Mvc/Property/TypeConverter/FormDefinitionArrayConverterTest.php
+++ b/typo3/sysext/form/Tests/Unit/Mvc/Property/TypeConverter/FormDefinitionArrayConverterTest.php
@@ -18,7 +18,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Form\Tests\Unit\Mvc\Property\TypeConverter;
use PHPUnit\Framework\Attributes\Test;
-use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Core\Crypto\HashService;
use TYPO3\CMS\Form\Domain\Configuration\Exception\PropertyException;
use TYPO3\CMS\Form\Domain\Configuration\FormDefinitionValidationService;
use TYPO3\CMS\Form\Mvc\Property\TypeConverter\FormDefinitionArrayConverter;
@@ -28,11 +28,13 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
final class FormDefinitionArrayConverterTest extends UnitTestCase
{
protected bool $resetSingletonInstances = true;
+ protected HashService $hashService;
public function setUp(): void
{
parent::setUp();
$GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = '12345';
+ $this->hashService = new HashService();
}
#[Test]
@@ -55,11 +57,11 @@ final class FormDefinitionArrayConverterTest extends UnitTestCase
],
'_orig_prototypeName' => [
'value' => 'standard',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
],
'_orig_identifier' => [
'value' => 'test',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'identifier', 'test']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'identifier', 'test']), $sessionToken),
],
];
@@ -170,11 +172,11 @@ final class FormDefinitionArrayConverterTest extends UnitTestCase
'identifier' => 'test',
'_orig_prototypeName' => [
'value' => 'standard',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
],
'_orig_identifier' => [
'value' => 'test',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'identifier', 'test']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'identifier', 'test']), $sessionToken),
],
];
@@ -199,11 +201,11 @@ final class FormDefinitionArrayConverterTest extends UnitTestCase
'identifier' => 'xxx',
'_orig_prototypeName' => [
'value' => 'standard',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'prototypeName', 'standard']), $sessionToken),
],
'_orig_identifier' => [
'value' => 'test',
- 'hmac' => GeneralUtility::hmac(serialize(['test', 'prototypeName', 'test']), $sessionToken),
+ 'hmac' => $this->hashService->hmac(serialize(['test', 'prototypeName', 'test']), $sessionToken),
],
];
--
GitLab