From bf958723c2eb072a39e9f3ff98eeb45552506a14 Mon Sep 17 00:00:00 2001
From: IanM <ian@morland.me>
Date: Thu, 4 Jan 2024 18:02:03 +0000
Subject: [PATCH] fix: prevent open redirects on logout controller

---
 .../src/Forum/Controller/LogOutController.php | 43 +++++++++++++++++--
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/framework/core/src/Forum/Controller/LogOutController.php b/framework/core/src/Forum/Controller/LogOutController.php
index f5eb88d30e..c9adcad1d8 100644
--- a/framework/core/src/Forum/Controller/LogOutController.php
+++ b/framework/core/src/Forum/Controller/LogOutController.php
@@ -9,6 +9,7 @@
 
 namespace Flarum\Forum\Controller;
 
+use Flarum\Foundation\Config;
 use Flarum\Http\Exception\TokenMismatchException;
 use Flarum\Http\Rememberer;
 use Flarum\Http\RequestUtil;
@@ -51,25 +52,33 @@ class LogOutController implements RequestHandlerInterface
      */
     protected $url;
 
+    /**
+     * @var Config
+     */
+    protected $config;
+
     /**
      * @param Dispatcher $events
      * @param SessionAuthenticator $authenticator
      * @param Rememberer $rememberer
      * @param Factory $view
      * @param UrlGenerator $url
+     * @param Config $config
      */
     public function __construct(
         Dispatcher $events,
         SessionAuthenticator $authenticator,
         Rememberer $rememberer,
         Factory $view,
-        UrlGenerator $url
+        UrlGenerator $url,
+        Config $config
     ) {
         $this->events = $events;
         $this->authenticator = $authenticator;
         $this->rememberer = $rememberer;
         $this->view = $view;
         $this->url = $url;
+        $this->config = $config;
     }
 
     /**
@@ -81,12 +90,13 @@ public function handle(Request $request): ResponseInterface
     {
         $session = $request->getAttribute('session');
         $actor = RequestUtil::getActor($request);
+        $base = $this->url->to('forum')->base();
 
-        $url = Arr::get($request->getQueryParams(), 'return', $this->url->to('forum')->base());
+        $url = Arr::get($request->getQueryParams(), 'return', $base);
 
         // If there is no user logged in, return to the index.
         if ($actor->isGuest()) {
-            return new RedirectResponse($url);
+            return new RedirectResponse($base);
         }
 
         // If a valid CSRF token hasn't been provided, show a view which will
@@ -94,7 +104,7 @@ public function handle(Request $request): ResponseInterface
         $csrfToken = $session->token();
 
         if (Arr::get($request->getQueryParams(), 'token') !== $csrfToken) {
-            $return = Arr::get($request->getQueryParams(), 'return');
+            $return = $this->sanitizeReturnUrl($request->getQueryParams()['return'] ?? $base);
 
             $view = $this->view->make('flarum.forum::log-out')
                 ->with('url', $this->url->to('forum')->route('logout').'?token='.$csrfToken.($return ? '&return='.urlencode($return) : ''));
@@ -113,4 +123,29 @@ public function handle(Request $request): ResponseInterface
 
         return $this->rememberer->forget($response);
     }
+
+    protected function sanitizeReturnUrl(string $url): string
+    {
+        $parsed = parse_url($url);
+
+        if (! $parsed || ! isset($parsed['host'])) {
+            return '';
+        }
+
+        $host = $parsed['host'];
+
+        if (in_array($host, $this->getWhitelistedRedirectDomains())) {
+            return $url;
+        }
+
+        return '';
+    }
+
+    protected function getWhitelistedRedirectDomains(): array
+    {
+        return array_merge(
+            [$this->config->url()],
+            $this->config->offsetGet('trustedHosts') ?? []
+        );
+    }
 }