From 65631dde4c46efa9fe69181c5a869f9473ed2ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Sun, 19 Jul 2026 23:05:02 +0200 Subject: [PATCH] Ensure that the Password reset link is generated on a trusted domain, and cannot be manipulated by an attacker by setting the host header --- src/Services/System/TrustedUrlGenerator.php | 96 ++++++++++ .../UserSystem/PasswordResetManager.php | 9 +- templates/mail/pw_reset.html.twig | 4 +- .../System/TrustedUrlGeneratorTest.php | 172 ++++++++++++++++++ 4 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 src/Services/System/TrustedUrlGenerator.php create mode 100644 tests/Services/System/TrustedUrlGeneratorTest.php diff --git a/src/Services/System/TrustedUrlGenerator.php b/src/Services/System/TrustedUrlGenerator.php new file mode 100644 index 00000000..3e023a47 --- /dev/null +++ b/src/Services/System/TrustedUrlGenerator.php @@ -0,0 +1,96 @@ +. + */ + +declare(strict_types=1); + +namespace App\Services\System; + +use Symfony\Component\DependencyInjection\Attribute\Autowire; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; + +/** + * Generates absolute URLs for routes whose host can be trusted, for use in contexts where the host must not + * be attacker-controllable (e.g. links sent out via email). + * + * If no TRUSTED_HOSTS is configured, Symfony does not validate the Host header of the incoming request in + * any way, so it must not be trusted to generate such links (otherwise an attacker could poison them via a + * forged Host header). In that case, the host/scheme/base path are forced to the ones configured via + * DEFAULT_URI instead of the ones from the current HTTP request. + * If TRUSTED_HOSTS is configured, Symfony already rejects requests with a non-matching Host header, so the + * request's host can be trusted and is used as usual (which allows the app to be reachable under multiple + * trusted hostnames). + */ +final class TrustedUrlGenerator +{ + public function __construct( + private readonly UrlGeneratorInterface $urlGenerator, + private readonly TrustedHostsChecker $trustedHostsChecker, + #[Autowire('%partdb.default_uri%')] + private readonly string $defaultUri, + ) { + } + + /** + * @param array $parameters + */ + public function generate(string $route, array $parameters = []): string + { + //If TRUSTED_HOSTS is configured, Symfony already validated the request's Host header, so it can be trusted + if (!$this->trustedHostsChecker->isTrustedHostsUnconfigured()) { + return $this->urlGenerator->generate($route, $parameters, UrlGeneratorInterface::ABSOLUTE_URL); + } + + $parts = parse_url(rtrim($this->defaultUri, '/')); + + $context = $this->urlGenerator->getContext(); + + //Backup the original context, so we can restore it after generating the URL + $original = [ + 'host' => $context->getHost(), + 'scheme' => $context->getScheme(), + 'httpPort' => $context->getHttpPort(), + 'httpsPort' => $context->getHttpsPort(), + 'baseUrl' => $context->getBaseUrl(), + ]; + + $scheme = $parts['scheme'] ?? 'https'; + $context->setScheme($scheme); + $context->setHost($parts['host'] ?? ''); + if (isset($parts['port'])) { + if ($scheme === 'https') { + $context->setHttpsPort($parts['port']); + } else { + $context->setHttpPort($parts['port']); + } + } + $context->setBaseUrl($parts['path'] ?? ''); + + try { + return $this->urlGenerator->generate($route, $parameters, UrlGeneratorInterface::ABSOLUTE_URL); + } finally { + //Restore the original context, so the rest of the request is not affected + $context->setHost($original['host']); + $context->setScheme($original['scheme']); + $context->setHttpPort($original['httpPort']); + $context->setHttpsPort($original['httpsPort']); + $context->setBaseUrl($original['baseUrl']); + } + } +} diff --git a/src/Services/UserSystem/PasswordResetManager.php b/src/Services/UserSystem/PasswordResetManager.php index 1a78cb60..d826703f 100644 --- a/src/Services/UserSystem/PasswordResetManager.php +++ b/src/Services/UserSystem/PasswordResetManager.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace App\Services\UserSystem; use App\Entity\UserSystem\User; +use App\Services\System\TrustedUrlGenerator; use DateTime; use Doctrine\ORM\EntityManagerInterface; use Symfony\Bridge\Twig\Mime\TemplatedEmail; @@ -39,7 +40,8 @@ class PasswordResetManager public function __construct(protected MailerInterface $mailer, protected EntityManagerInterface $em, protected TranslatorInterface $translator, protected UserPasswordHasherInterface $userPasswordEncoder, - PasswordHasherFactoryInterface $encoderFactory) + PasswordHasherFactoryInterface $encoderFactory, + protected TrustedUrlGenerator $trustedUrlGenerator) { $this->passwordEncoder = $encoderFactory->getPasswordHasher(User::class); } @@ -63,6 +65,9 @@ class PasswordResetManager $user->setPwResetExpires($expiration_date); if ($user->getEmail() !== null && $user->getEmail() !== '') { + $reset_url = $this->trustedUrlGenerator->generate('pw_reset_new_pw', ['user' => $user->getName(), 'token' => $unencrypted_token]); + $reset_url_fallback = $this->trustedUrlGenerator->generate('pw_reset_new_pw'); + $address = new Address($user->getEmail(), $user->getFullName()); $mail = new TemplatedEmail(); $mail->to($address); @@ -72,6 +77,8 @@ class PasswordResetManager 'expiration_date' => $expiration_date, 'token' => $unencrypted_token, 'user' => $user, + 'reset_url' => $reset_url, + 'reset_url_fallback' => $reset_url_fallback, ]); //Send email diff --git a/templates/mail/pw_reset.html.twig b/templates/mail/pw_reset.html.twig index 18a867d6..f9b9662c 100644 --- a/templates/mail/pw_reset.html.twig +++ b/templates/mail/pw_reset.html.twig @@ -6,9 +6,9 @@

{% trans with {'%name%': user.fullName|escape } %}email.hi %name%{% endtrans %},

{% trans %}email.pw_reset.message{% endtrans %}
- +
- {% trans with {'%url%': url('pw_reset_new_pw') } %}email.pw_reset.fallback{% endtrans %}: + {% trans with {'%url%': reset_url_fallback } %}email.pw_reset.fallback{% endtrans %}: diff --git a/tests/Services/System/TrustedUrlGeneratorTest.php b/tests/Services/System/TrustedUrlGeneratorTest.php new file mode 100644 index 00000000..40c7c2c5 --- /dev/null +++ b/tests/Services/System/TrustedUrlGeneratorTest.php @@ -0,0 +1,172 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Services\System; + +use App\Services\System\TrustedHostsChecker; +use App\Services\System\TrustedUrlGenerator; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\Component\Routing\RequestContext; + +final class TrustedUrlGeneratorTest extends TestCase +{ + private function createGenerator(UrlGeneratorInterface $urlGenerator, string $trustedHosts, string $defaultUri): TrustedUrlGenerator + { + return new TrustedUrlGenerator($urlGenerator, new TrustedHostsChecker($trustedHosts), $defaultUri); + } + + public function testUsesCurrentRequestContextWhenTrustedHostsIsConfigured(): void + { + //If TRUSTED_HOSTS is configured, the request's host is already validated by Symfony, so it should be used as-is + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->expects($this->never())->method('getContext'); + $urlGenerator->expects($this->once()) + ->method('generate') + ->with('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc'], UrlGeneratorInterface::ABSOLUTE_URL) + ->willReturn('https://request-host.example.com/pw_reset/new_pw/john/abc'); + + $generator = $this->createGenerator($urlGenerator, 'example\.com', 'https://trusted.example.com/'); + + $url = $generator->generate('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc']); + $this->assertSame('https://request-host.example.com/pw_reset/new_pw/john/abc', $url); + } + + public function testForcesDefaultUriHostWhenTrustedHostsIsNotConfigured(): void + { + //If TRUSTED_HOSTS is not configured, the request's Host header is attacker-controllable, so the + //host/scheme/base path configured via DEFAULT_URI must be used instead. + $context = new RequestContext(); + $context->setScheme('http'); + $context->setHost('attacker.evil'); + $context->setBaseUrl(''); + + $capturedHost = null; + $capturedScheme = null; + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->method('getContext')->willReturn($context); + $urlGenerator->expects($this->once()) + ->method('generate') + ->with('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc'], UrlGeneratorInterface::ABSOLUTE_URL) + ->willReturnCallback(function () use ($context, &$capturedHost, &$capturedScheme) { + //Capture the context state as it is seen by the URL generator during the call + $capturedHost = $context->getHost(); + $capturedScheme = $context->getScheme(); + + return $capturedScheme.'://'.$capturedHost.'/pw_reset/new_pw/john/abc'; + }); + + $generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/'); + + $url = $generator->generate('pw_reset_new_pw', ['user' => 'john', 'token' => 'abc']); + + $this->assertSame('trusted.example.com', $capturedHost); + $this->assertSame('https', $capturedScheme); + $this->assertSame('https://trusted.example.com/pw_reset/new_pw/john/abc', $url); + } + + public function testRestoresOriginalContextAfterGenerating(): void + { + //The context is shared with the rest of the application, so it must not be left modified afterward + $context = new RequestContext(); + $context->setScheme('http'); + $context->setHost('attacker.evil'); + $context->setHttpPort(8080); + $context->setBaseUrl('/original'); + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->method('getContext')->willReturn($context); + $urlGenerator->method('generate')->willReturn('https://trusted.example.com/foo'); + + $generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/sub/'); + $generator->generate('some_route'); + + $this->assertSame('attacker.evil', $context->getHost()); + $this->assertSame('http', $context->getScheme()); + $this->assertSame(8080, $context->getHttpPort()); + $this->assertSame('/original', $context->getBaseUrl()); + } + + public function testRestoresOriginalContextEvenWhenGenerateThrows(): void + { + $context = new RequestContext(); + $context->setScheme('http'); + $context->setHost('attacker.evil'); + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->method('getContext')->willReturn($context); + $urlGenerator->method('generate')->willThrowException(new \RuntimeException('route not found')); + + $generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/'); + + try { + $generator->generate('unknown_route'); + $this->fail('Expected exception was not thrown'); + } catch (\RuntimeException) { + //Expected + } + + $this->assertSame('attacker.evil', $context->getHost()); + $this->assertSame('http', $context->getScheme()); + } + + public function testUsesHttpsPortForHttpsDefaultUri(): void + { + $context = new RequestContext(); + + $capturedHttpsPort = null; + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->method('getContext')->willReturn($context); + $urlGenerator->method('generate')->willReturnCallback(function () use ($context, &$capturedHttpsPort) { + $capturedHttpsPort = $context->getHttpsPort(); + + return 'https://trusted.example.com:8443/foo'; + }); + + $generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com:8443/'); + $generator->generate('some_route'); + + $this->assertSame(8443, $capturedHttpsPort); + } + + public function testUsesBasePathFromDefaultUri(): void + { + $context = new RequestContext(); + + $capturedBaseUrl = null; + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $urlGenerator->method('getContext')->willReturn($context); + $urlGenerator->method('generate')->willReturnCallback(function () use ($context, &$capturedBaseUrl) { + $capturedBaseUrl = $context->getBaseUrl(); + + return 'https://trusted.example.com/partdb/foo'; + }); + + $generator = $this->createGenerator($urlGenerator, '', 'https://trusted.example.com/partdb/'); + $generator->generate('some_route'); + + $this->assertSame('/partdb', $capturedBaseUrl); + } +}