Skip to content

Commit 8639ef9

Browse files
authored
[5.x] Harden redirects (#14099)
1 parent 82774b0 commit 8639ef9

4 files changed

Lines changed: 63 additions & 3 deletions

File tree

src/Exceptions/Concerns/RendersControlPanelExceptions.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Statamic\Exceptions\Concerns;
44

55
use Illuminate\Auth\Access\AuthorizationException as IlluminateAuthException;
6+
use Statamic\Facades\URL;
67

78
trait RendersControlPanelExceptions
89
{
@@ -21,7 +22,7 @@ protected function getAuthExceptionRedirectUrl()
2122

2223
// If we came to this URL from another, we'll send them back, but not
2324
// if it was the login page otherwise there'd be a redirect loop.
24-
if ($referrer && $referrer != cp_route('login')) {
25+
if ($referrer && $referrer != cp_route('login') && ! URL::isExternalToApplication($referrer)) {
2526
return $referrer;
2627
}
2728

src/Http/Controllers/FormController.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ private function formFailure($params, $errors, $form)
124124

125125
$redirect = Arr::get($params, '_error_redirect');
126126

127-
$response = $redirect ? redirect($redirect) : back();
127+
$response = $redirect && ! \Statamic\Facades\URL::isExternalToApplication($redirect)
128+
? redirect($redirect)
129+
: back();
128130

129131
return $response->withInput()->withErrors($errors, 'form.'.$form);
130132
}
@@ -152,7 +154,9 @@ private function formSuccess($params, $submission, $silentFailure = false)
152154
]);
153155
}
154156

155-
$response = $redirect ? redirect($redirect) : back();
157+
$response = $redirect && ! \Statamic\Facades\URL::isExternalToApplication($redirect)
158+
? redirect($redirect)
159+
: back();
156160

157161
if (! \Statamic\Facades\URL::isExternal($redirect)) {
158162
session()->flash("form.{$submission->form()->handle()}.success", __('Submission successful.'));

tests/CP/AuthRedirectTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ public function it_redirects_somewhere_if_the_referrer_was_the_login_page()
6868
->assertSessionHas(['error' => "Can't touch this."]);
6969
}
7070

71+
#[Test]
72+
public function it_does_not_redirect_to_external_referrer()
73+
{
74+
$this->setTestRoles(['test' => ['access cp']]);
75+
$user = tap(User::make()->assignRole('test'))->save();
76+
77+
$this
78+
->actingAs($user)
79+
->withHeaders(['referer' => 'https://external.com'])
80+
->get('/cp/hammertime')
81+
->assertRedirect(cp_route('index'))
82+
->assertSessionHas(['error' => "Can't touch this."]);
83+
}
84+
7185
#[Test]
7286
public function it_redirects_to_unauthorized_view_if_there_would_be_a_redirect_loop()
7387
{

tests/Tags/Form/FormCreateTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,24 @@ public function it_will_submit_form_and_follow_custom_redirect_with_success()
677677
$this->assertStringContainsString('<div class="analytics"></div>', $output);
678678
}
679679

680+
#[Test]
681+
public function it_does_not_follow_external_redirect_on_success()
682+
{
683+
$this->assertEmpty(Form::find('contact')->submissions());
684+
685+
$this
686+
->from('/contact')
687+
->post('/!/forms/contact', [
688+
'email' => 'san@holo.com',
689+
'message' => 'hello',
690+
'_redirect' => 'https://evil.com/phishing',
691+
])
692+
->assertSessionHasNoErrors()
693+
->assertLocation('/contact');
694+
695+
$this->assertCount(1, Form::find('contact')->submissions());
696+
}
697+
680698
#[Test]
681699
public function it_will_submit_form_with_honeypot_filled_and_render_fake_success()
682700
{
@@ -753,6 +771,29 @@ public function it_wont_submit_form_and_follow_custom_redirect_with_errors()
753771
$this->assertEmpty($success[1]);
754772
}
755773

774+
#[Test]
775+
public function it_does_not_follow_external_error_redirect()
776+
{
777+
$this->assertEmpty(Form::find('contact')->submissions());
778+
779+
Event::listen(function (\Statamic\Events\FormSubmitted $event) {
780+
throw ValidationException::withMessages(['custom' => 'This is a custom message']);
781+
});
782+
783+
$this
784+
->from('/contact')
785+
->post('/!/forms/contact', [
786+
'_error_redirect' => 'https://evil.com/phishing',
787+
'name' => 'Hansolo',
788+
'email' => 'san@holo.com',
789+
'message' => 'hello',
790+
])
791+
->assertSessionHasErrors(['custom'], null, 'form.contact')
792+
->assertLocation('/contact');
793+
794+
$this->assertCount(0, Form::find('contact')->submissions());
795+
}
796+
756797
#[Test]
757798
public function it_will_use_redirect_query_param_off_url()
758799
{

0 commit comments

Comments
 (0)