bug #62487 [Security] Fix UserBadge validation bypass via identifier normalizer (yoeunes)

nicolas-grekas · nicolas-grekas · commit bbc8aab6ad43 · 2025-11-24T10:31:07.000+01:00
This PR was merged into the 7.3 branch. Discussion ---------- [Security] Fix `UserBadge` validation bypass via identifier normalizer | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT The `UserBadge` constructor validates that the identifier is not empty and does not exceed `MAX_USERNAME_LENGTH`. However, when using `$identifierNormalizer`, the normalized identifier is computed lazily in `getUserIdentifier()` without validation. This allows normalizers to return invalid values: ```php // This correctly triggers a deprecation in the constructor new UserBadge(''); // This currently bypasses validation and returns an empty string $badge = new UserBadge('valid_input', null, null, fn() => ''); $badge->getUserIdentifier(); ``` Related to #51744 and #61183 I targeted `7.3` as it introduced `identifierNormalizer`, please let me know if I should target `8.0` or `8.1` instead. Commits ------- e4a759d [Security] Fix UserBadge validation bypass via identifier normalizer
diff --git a/src/Symfony/Component/Security/Http/Authenticator/Passport/Badge/UserBadge.php b/src/Symfony/Component/Security/Http/Authenticator/Passport/Badge/UserBadge.php
@@ -54,14 +54,8 @@ public function __construct(
         private ?array $attributes = null,
         ?\Closure $identifierNormalizer = null,
     ) {
-        if ('' === $userIdentifier) {
-            trigger_deprecation('symfony/security-http', '7.2', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
-            // throw new BadCredentialsException('Empty user identifier.');
-        }
+        $this->validateUserIdentifier($userIdentifier);
 
-        if (\strlen($userIdentifier) > self::MAX_USERNAME_LENGTH) {
-            throw new BadCredentialsException('Username too long.');
-        }
         if ($identifierNormalizer) {
             $this->identifierNormalizer = static fn () => $identifierNormalizer($userIdentifier);
         }
@@ -74,6 +68,8 @@ public function getUserIdentifier(): string
         if (isset($this->identifierNormalizer)) {
             $this->userIdentifier = ($this->identifierNormalizer)();
             $this->identifierNormalizer = null;
+
+            $this->validateUserIdentifier($this->userIdentifier);
         }
 
         return $this->userIdentifier;
@@ -132,4 +128,16 @@ public function isResolved(): bool
     {
         return true;
     }
+
+    private function validateUserIdentifier(string $userIdentifier): void
+    {
+        if ('' === $userIdentifier) {
+            trigger_deprecation('symfony/security-http', '7.2', 'Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
+            // throw new BadCredentialsException('Empty user identifier.');
+        }
+
+        if (\strlen($userIdentifier) > self::MAX_USERNAME_LENGTH) {
+            throw new BadCredentialsException('Username too long.');
+        }
+    }
 }
diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/Passport/Badge/UserBadgeTest.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/Passport/Badge/UserBadgeTest.php
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait;
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\Exception\UserNotFoundException;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\String\Slugger\AsciiSlugger;
@@ -69,4 +70,26 @@ public static function provideUserIdentifierNormalizationData(): iterable
         yield 'Greek to ASCII' => ['ΝιΚόΛΑος', 'NIKOLAOS', $upperAndAscii];
         yield 'Katakana to ASCII' => ['たなかそういち', 'TANAKASOUICHI', $upperAndAscii];
     }
+
+    /**
+     * @group legacy
+     */
+    public function testUserIdentifierNormalizationTriggersDeprecationForEmptyString()
+    {
+        $badge = new UserBadge('valid_input', null, null, fn () => '');
+
+        $this->expectUserDeprecationMessage('Since symfony/security-http 7.2: Using an empty string as user identifier is deprecated and will throw an exception in Symfony 8.0.');
+
+        $this->assertSame('', $badge->getUserIdentifier());
+    }
+
+    public function testUserIdentifierNormalizationEnforcesMaxLength()
+    {
+        $badge = new UserBadge('valid_input', null, null, fn () => str_repeat('a', UserBadge::MAX_USERNAME_LENGTH + 1));
+
+        $this->expectException(BadCredentialsException::class);
+        $this->expectExceptionMessage('Username too long.');
+
+        $badge->getUserIdentifier();
+    }
 }
