Skip to content

Commit 51cd022

Browse files
jasonvargajasonvarga
authored
[6.x] Harden password reset (#14294)
1 parent 466b2e8 commit 51cd022

File tree

5 files changed

+278
-89
lines changed

5 files changed

+278
-89
lines changed

‎src/Auth/UserTags.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ public function forgotPasswordForm()
352352
$params['error_redirect'] = $this->parseRedirect($errorRedirect);
353353
}
354354

355-
if ($resetUrl = $this->params->get('reset_url')) {
355+
if ($resetUrl = $this->getPasswordResetUrl($this->params->get('reset_url'))) {
356356
$params['reset_url'] = $resetUrl;
357357
}
358358

@@ -374,6 +374,19 @@ public function forgotPasswordForm()
374374
return $html;
375375
}
376376

377+
private function getPasswordResetUrl(?string $url = null): ?string
378+
{
379+
if (! $url) {
380+
return null;
381+
}
382+
383+
if (! URL::isAbsolute($url) && ! str_starts_with($url, '/')) {
384+
$url = '/'.$url;
385+
}
386+
387+
return encrypt($url);
388+
}
389+
377390
/**
378391
* Output a reset password form.
379392
*

‎src/Http/Controllers/ForgotPasswordController.php‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
namespace Statamic\Http\Controllers;
44

5+
use Illuminate\Contracts\Encryption\DecryptException;
56
use Illuminate\Http\Request;
67
use Illuminate\Support\Facades\Password;
78
use Inertia\Inertia;
89
use Statamic\Auth\Passwords\PasswordReset;
910
use Statamic\Auth\SendsPasswordResetEmails;
10-
use Statamic\Exceptions\ValidationException;
1111
use Statamic\Facades\URL;
1212
use Statamic\Http\Middleware\RedirectIfAuthenticated;
1313

@@ -32,17 +32,40 @@ public function showLinkRequestForm()
3232

3333
public function sendResetLinkEmail(Request $request)
3434
{
35-
if ($url = $request->_reset_url) {
36-
throw_if(URL::isExternalToApplication($url), ValidationException::withMessages([
37-
'_reset_url' => trans('validation.url', ['attribute' => '_reset_url']),
38-
]));
39-
35+
if ($url = $this->getResetFormUrl($request)) {
4036
PasswordReset::resetFormUrl(URL::makeAbsolute($url));
4137
}
4238

4339
return $this->traitSendResetLinkEmail($request);
4440
}
4541

42+
private function getResetFormUrl(Request $request): ?string
43+
{
44+
if (! $url = $request->_reset_url) {
45+
return null;
46+
}
47+
48+
if (strlen($url) > 2048) {
49+
return null;
50+
}
51+
52+
try {
53+
$url = decrypt($url);
54+
} catch (DecryptException $e) {
55+
if (! str_starts_with($url, '/') || str_starts_with($url, '//')) {
56+
return null;
57+
}
58+
59+
if (preg_match('/[\x00-\x1F\x7F]/', $url)) {
60+
return null;
61+
}
62+
63+
return $url;
64+
}
65+
66+
return URL::isExternalToApplication($url) ? null : $url;
67+
}
68+
4669
public function broker()
4770
{
4871
$broker = config('statamic.users.passwords.'.PasswordReset::BROKER_RESETS);

‎tests/Auth/ForgotPasswordTest.php‎

Lines changed: 120 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Illuminate\Support\Facades\Password;
66
use PHPUnit\Framework\Attributes\DataProvider;
77
use PHPUnit\Framework\Attributes\Test;
8+
use Statamic\Auth\Passwords\PasswordReset;
89
use Statamic\Facades\User;
910
use Tests\Facades\Concerns\ProvidesExternalUrls;
1011
use Tests\PreventSavingStacheItemsToDisk;
@@ -26,6 +27,8 @@ public function setUp(): void
2627
{
2728
parent::setUp();
2829

30+
PasswordReset::resetFormUrl(null);
31+
2932
$this->setSites([
3033
'a' => ['name' => 'A', 'locale' => 'en_US', 'url' => 'http://this-site.com/'],
3134
'b' => ['name' => 'B', 'locale' => 'en_US', 'url' => 'http://subdomain.this-site.com/'],
@@ -34,28 +37,132 @@ public function setUp(): void
3437
}
3538

3639
#[Test]
37-
#[DataProvider('externalUrlProvider')]
38-
public function it_validates_reset_url_when_sending_reset_link_email($url, $isExternal)
40+
public function it_accepts_encrypted_reset_url_when_sending_reset_link_email()
3941
{
4042
$this->simulateSuccessfulPasswordResetEmail();
43+
$this->createUser();
4144

42-
User::make()
43-
->email('san@holo.com')
44-
->password('chewy')
45-
->save();
45+
$this->post('/!/auth/password/email', [
46+
'email' => 'san@holo.com',
47+
'_reset_url' => encrypt('http://this-site.com/some-path'),
48+
])->assertSessionHasNoErrors();
49+
$this->assertEquals('http://this-site.com/some-path?token=test-token', PasswordReset::url('test-token', 'resets'));
50+
}
4651

47-
$response = $this->post('/!/auth/password/email', [
52+
#[Test]
53+
public function it_accepts_unencrypted_relative_reset_url_when_sending_reset_link_email()
54+
{
55+
$this->simulateSuccessfulPasswordResetEmail();
56+
$this->createUser();
57+
58+
$this->post('/!/auth/password/email', [
59+
'email' => 'san@holo.com',
60+
'_reset_url' => '/some-path',
61+
])->assertSessionHasNoErrors();
62+
$this->assertEquals('http://absolute-url-resolved-from-request.com/some-path?token=test-token', PasswordReset::url('test-token', 'resets'));
63+
}
64+
65+
#[Test]
66+
#[DataProvider('externalResetUrlProvider')]
67+
public function it_rejects_unencrypted_external_reset_url_when_sending_reset_link_email($url)
68+
{
69+
$this->simulateSuccessfulPasswordResetEmail();
70+
$this->createUser();
71+
72+
$this->post('/!/auth/password/email', [
4873
'email' => 'san@holo.com',
4974
'_reset_url' => $url,
50-
]);
75+
])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url.
76+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
77+
}
78+
79+
#[Test]
80+
public function it_rejects_unencrypted_absolute_internal_reset_url_when_sending_reset_link_email()
81+
{
82+
$this->simulateSuccessfulPasswordResetEmail();
83+
$this->createUser();
84+
85+
$this->post('/!/auth/password/email', [
86+
'email' => 'san@holo.com',
87+
'_reset_url' => 'http://this-site.com/some-path',
88+
])->assertSessionHasNoErrors();
89+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
90+
}
5191

52-
if ($isExternal) {
53-
$response->assertSessionHasErrors(['_reset_url']);
92+
#[Test]
93+
public function it_rejects_unencrypted_relative_reset_url_with_control_characters_when_sending_reset_link_email()
94+
{
95+
$this->simulateSuccessfulPasswordResetEmail();
96+
$this->createUser();
5497

55-
return;
56-
}
98+
$this->post('/!/auth/password/email', [
99+
'email' => 'san@holo.com',
100+
'_reset_url' => "/some-path\r\nLocation: https://evil.com",
101+
])->assertSessionHasNoErrors();
102+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
103+
}
57104

58-
$response->assertSessionHasNoErrors();
105+
#[Test]
106+
public function it_rejects_reset_url_longer_than_2048_characters_when_sending_reset_link_email()
107+
{
108+
$this->simulateSuccessfulPasswordResetEmail();
109+
$this->createUser();
110+
111+
$this->post('/!/auth/password/email', [
112+
'email' => 'san@holo.com',
113+
'_reset_url' => '/'.str_repeat('a', 2048),
114+
])->assertSessionHasNoErrors();
115+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
116+
}
117+
118+
#[Test]
119+
public function it_rejects_unencrypted_string_reset_url_when_sending_reset_link_email()
120+
{
121+
// Unencrypted string that doesn't look like a URL is probably a tampered encrypted string.
122+
// It might be a relative url without a leading slash, but we won't treat it as that.
123+
124+
$this->simulateSuccessfulPasswordResetEmail();
125+
$this->createUser();
126+
127+
$this->post('/!/auth/password/email', [
128+
'email' => 'san@holo.com',
129+
'_reset_url' => 'not-an-encrypted-string',
130+
])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url.
131+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
132+
}
133+
134+
#[Test]
135+
#[DataProvider('externalResetUrlProvider')]
136+
public function it_rejects_encrypted_external_reset_url_when_sending_reset_link_email($url)
137+
{
138+
// It's weird to point to an external URL, even if you encrypt it yourself.
139+
// This is an additional safeguard.
140+
141+
$this->simulateSuccessfulPasswordResetEmail();
142+
$this->createUser();
143+
144+
$this->post('/!/auth/password/email', [
145+
'email' => 'san@holo.com',
146+
'_reset_url' => encrypt($url),
147+
])->assertSessionHasNoErrors(); // Allow the notification to be sent, but without the bad url.
148+
$this->assertEquals('http://absolute-url-resolved-from-request.com/!/auth/password/reset/test-token?', PasswordReset::url('test-token', 'resets'));
149+
}
150+
151+
public static function externalResetUrlProvider()
152+
{
153+
$keyFn = function ($key) {
154+
return is_null($key) ? 'null' : $key;
155+
};
156+
157+
return collect(static::externalUrls())->mapWithKeys(fn ($url) => [$keyFn($url) => [$url]])->all();
158+
}
159+
160+
private function createUser(): void
161+
{
162+
User::make()
163+
->email('san@holo.com')
164+
->password('chewy')
165+
->save();
59166
}
60167

61168
protected function simulateSuccessfulPasswordResetEmail()

0 commit comments

Comments
 (0)