From 6e7cc169d1527c18556d536159e0e23df85c381e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 10 Mar 2026 18:31:51 +0000 Subject: [PATCH] Preferences: Updated return redirect with better origin checks As suggested by Alex Dan in their security report. --- app/Http/Controller.php | 16 ++++++++++++++-- tests/User/UserPreferencesTest.php | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/Http/Controller.php b/app/Http/Controller.php index 5d3be4951..1a0f5932e 100644 --- a/app/Http/Controller.php +++ b/app/Http/Controller.php @@ -167,14 +167,26 @@ abstract class Controller extends BaseController /** * Redirect to the URL provided in the request as a '_return' parameter. - * Will check that the parameter leads to a URL under the root path of the system. + * Will check that the parameter leads to a URL under the same origin as the application. */ protected function redirectToRequest(Request $request): RedirectResponse { $basePath = url('/'); $returnUrl = $request->input('_return') ?? $basePath; - if (!str_starts_with($returnUrl, $basePath)) { + // Only allow use of _return on requests where we expect CSRF to be active + // to prevent it potentially being used as an open redirect + $allowedMethods = ['POST', 'PUT', 'PATCH', 'DELETE']; + if (!in_array($request->getMethod(), $allowedMethods)) { + return redirect($basePath); + } + + $intendedUrl = parse_url($returnUrl); + $baseUrl = parse_url($basePath); + $isSameOrigin = ($intendedUrl['host'] ?? '') === ($baseUrl['host'] ?? '') + && ($intendedUrl['scheme'] ?? '') === ($baseUrl['scheme'] ?? '') + && ($intendedUrl['port'] ?? 0) === ($baseUrl['port'] ?? 0); + if (!$isSameOrigin) { return redirect($basePath); } diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index ff3cb63ca..e893f002d 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -153,6 +153,26 @@ class UserPreferencesTest extends TestCase ->assertElementNotExists('.content-wrap .entity-list-item'); } + public function test_redirect_on_preference_change_checks_host() + { + $expectedByRedirect = [ + 'http://localhost/beans' => 'http://localhost/beans', + 'https://localhost/beans' => 'http://localhost', + 'http://localhost:9090/beans' => 'http://localhost', + 'http://localhost.example.com/beans' => 'http://localhost', + 'http://localhost@example.com/beans' => 'http://localhost', + ]; + + $this->asEditor(); + foreach ($expectedByRedirect as $url => $expected) { + $req = $this->patch("/preferences/change-view/bookshelf", [ + 'view' => 'grid', + '_return' => $url, + ]); + $req->assertRedirect($expected); + } + } + public function test_update_code_language_favourite() { $editor = $this->users->editor();