From 83a6090d55ac3ef8a6f656ec3dc22f0bf88d413e Mon Sep 17 00:00:00 2001
From: Oliver Bartsch <bo@cedev.de>
Date: Mon, 7 Nov 2022 23:23:10 +0100
Subject: [PATCH] [FOLLOWUP][BUGFIX] Prevent out of bounds exception in
password generation
This followup prevents an out of bounds exception and
adds a couple of tests for the new functionality.
The min length is now set to 8. This is now also checked
when using the special "random" option.
Resolves: #98540
Releases: main
Change-Id: I5e89813794a0178b7d9c835e89c6272128b4b8c3
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/76461
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Oliver Bartsch <bo@cedev.de>
---
typo3/sysext/core/Classes/Crypto/Random.php | 21 ++-
...98540-NewFieldControlPasswordGenerator.rst | 2 +-
.../core/Tests/Unit/Crypto/RandomTest.php | 147 ++++++++++++++++++
3 files changed, 162 insertions(+), 8 deletions(-)
diff --git a/typo3/sysext/core/Classes/Crypto/Random.php b/typo3/sysext/core/Classes/Crypto/Random.php
index 8de171374cde..bd21b55bb283 100644
--- a/typo3/sysext/core/Classes/Crypto/Random.php
+++ b/typo3/sysext/core/Classes/Crypto/Random.php
@@ -74,14 +74,21 @@ class Random
*/
public function generateRandomPassword(array $passwordRules): string
{
- $password = '';
$passwordLength = (int)($passwordRules['length'] ?? self::DEFAULT_PASSWORD_LEGNTH);
+ if ($passwordLength < 8) {
+ throw new InvalidPasswordRulesException(
+ 'Password rules are invalid. Length must be at least 8.',
+ 1667557900
+ );
+ }
+
+ $password = '';
if ($passwordRules['random'] ?? false) {
$password = match ((string)$passwordRules['random']) {
'hex' => $this->generateRandomHexString($passwordLength),
'base64' => $this->generateRandomBase64String($passwordLength),
- default => throw new InvalidPasswordRulesException('Invalid value for special option \'random\'. Valid options are: \'hex\' and \'base64\'', 1667557901),
+ default => throw new InvalidPasswordRulesException('Invalid value for special password rule \'random\'. Valid options are: \'hex\' and \'base64\'', 1667557901),
};
} else {
$characters = [];
@@ -103,20 +110,20 @@ class Random
$characterSets[] = self::SPECIAL_CHARACTERS;
}
- if ($passwordLength <= 0 || $characterSets === []) {
+ if ($characterSets === []) {
throw new InvalidPasswordRulesException(
- 'Password rules are invalid. Length must be grater 0 and at least one character set must be allowed.',
- 1667557900
+ 'Password rules are invalid. At least one character set must be allowed.',
+ 1667557902
);
}
foreach ($characterSets as $characterSet) {
- $password .= $characterSet[random_int(0, strlen($characterSet))];
+ $password .= $characterSet[random_int(0, strlen($characterSet) - 1)];
}
$charactersCount = count($characters);
for ($i = 0; $i < $passwordLength - count($characterSets); $i++) {
- $password .= $characters[random_int(0, $charactersCount)];
+ $password .= $characters[random_int(0, $charactersCount - 1)];
}
str_shuffle($password);
diff --git a/typo3/sysext/core/Documentation/Changelog/12.1/Feature-98540-NewFieldControlPasswordGenerator.rst b/typo3/sysext/core/Documentation/Changelog/12.1/Feature-98540-NewFieldControlPasswordGenerator.rst
index a494758c4a05..de5d3abcc258 100644
--- a/typo3/sysext/core/Documentation/Changelog/12.1/Feature-98540-NewFieldControlPasswordGenerator.rst
+++ b/typo3/sysext/core/Documentation/Changelog/12.1/Feature-98540-NewFieldControlPasswordGenerator.rst
@@ -61,7 +61,7 @@ Field control options
Available password rules:
-- :php:`length`: Defines the amout of characters for the password (Default: :php:`16`).
+- :php:`length`: Defines the amout of characters for the password (minimum: :php:`8` - default: :php:`16`).
- :php:`random`: Defines the encoding of random bytes. Overrules charater definitions. Available encodings are :php:`hex` and :php:`base64`.
- :php:`digitCharacters`: Whether digits should be used (Default: :php:`true`)
- :php:`lowerCaseCharacters`: Whether lowercase characters should be used (Default: :php:`true`)
diff --git a/typo3/sysext/core/Tests/Unit/Crypto/RandomTest.php b/typo3/sysext/core/Tests/Unit/Crypto/RandomTest.php
index abd96b84c649..efa9eef87b4a 100644
--- a/typo3/sysext/core/Tests/Unit/Crypto/RandomTest.php
+++ b/typo3/sysext/core/Tests/Unit/Crypto/RandomTest.php
@@ -18,6 +18,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Core\Tests\Unit\Crypto;
use TYPO3\CMS\Core\Crypto\Random;
+use TYPO3\CMS\Core\Exception\InvalidPasswordRulesException;
use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
class RandomTest extends UnitTestCase
@@ -67,4 +68,150 @@ class RandomTest extends UnitTestCase
$subject = new Random();
self::assertEquals($numberOfChars, strlen($subject->generateRandomHexString($numberOfChars)));
}
+
+ public function generateRandomPasswordThrowsInvalidPasswordRulesExceptionDataProvider(): \Generator
+ {
+ yield 'Invalid length' => [
+ [
+ 'length' => 4,
+ ],
+ 1667557900,
+ ];
+ yield 'Invalid random value' => [
+ [
+ 'random' => 'invalid',
+ ],
+ 1667557901,
+ ];
+ yield 'Invalid characters definition' => [
+ [
+ 'lowerCaseCharacters' => false,
+ 'upperCaseCharacters' => false,
+ 'digitCharacters' => false,
+ ],
+ 1667557902,
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider generateRandomPasswordThrowsInvalidPasswordRulesExceptionDataProvider
+ */
+ public function generateRandomPasswordThrowsInvalidPasswordRulesException(
+ array $passwordRules,
+ int $exceptionCode
+ ): void {
+ $this->expectException(InvalidPasswordRulesException::class);
+ $this->expectExceptionCode($exceptionCode);
+
+ (new Random())->generateRandomPassword($passwordRules);
+ }
+
+ public function generateRandomPasswordGeneratesRandomWithEncodingDataProvider(): \Generator
+ {
+ yield 'Hex with 42 chars' => [
+ [
+ 'length' => 42,
+ 'random' => 'hex',
+ ],
+ '/^[a-fA-F0-9]{42}$/',
+ ];
+ yield 'Base64 with 37 chars' => [
+ [
+ 'length' => 37,
+ 'random' => 'base64',
+ 'digitCharacters' => false, // Won't be evaluated
+ ],
+ '/^[a-zA-Z0-9\-\_]{37}$/',
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider generateRandomPasswordGeneratesRandomWithEncodingDataProvider
+ */
+ public function generateRandomPasswordGeneratesRandomWithEncoding(
+ array $passwordRules,
+ string $pattern
+ ): void {
+ self::assertMatchesRegularExpression($pattern, (new Random())->generateRandomPassword($passwordRules));
+ }
+
+ public function generateRandomPasswordGeneratesRandomWithCharacterSetsDataProvider(): \Generator
+ {
+ yield 'lowercase' => [
+ [
+ 'lowerCaseCharacters' => true,
+ ],
+ '/[a-z]+/',
+ ];
+ yield 'uppercase' => [
+ [
+ 'upperCaseCharacters' => true,
+ ],
+ '/[A-Z]+/',
+ ];
+ yield 'digits' => [
+ [
+ 'digitCharacters' => true,
+ ],
+ '/[0-9]+/',
+ ];
+ yield 'special' => [
+ [
+ 'specialCharacters' => true,
+ ],
+ '/[\'!"#$%&()*+,\-.\/:;<=>?@\[\]^_`{|}~]+/',
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider generateRandomPasswordGeneratesRandomWithCharacterSetsDataProvider
+ */
+ public function generateRandomPasswordGeneratesRandomWithCharacterSets(
+ array $passwordRules,
+ string $pattern
+ ): void {
+ self::assertMatchesRegularExpression($pattern, (new Random())->generateRandomPassword($passwordRules));
+ }
+
+ public function generateRandomPasswordGeneratesRandomWithLengthDataProvider(): \Generator
+ {
+ yield 'fallback' => [
+ [],
+ 16,
+ ];
+ yield 'length=40' => [
+ [
+ 'length' => 40,
+ ],
+ 40,
+ ];
+ yield 'length=36 with random=hex' => [
+ [
+ 'length' => 36,
+ 'random' => 'hex',
+ ],
+ 36,
+ ];
+ yield 'length=42 with random=hex' => [
+ [
+ 'length' => 42,
+ 'random' => 'base64',
+ ],
+ 42,
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider generateRandomPasswordGeneratesRandomWithLengthDataProvider
+ */
+ public function generateRandomPasswordGeneratesRandomWithLength(
+ array $passwordRules,
+ int $length
+ ): void {
+ self::assertEquals($length, strlen((new Random())->generateRandomPassword($passwordRules)));
+ }
}
--
GitLab