[1.x] fix: Logout controller allows open redirects (#3948)

* fix: prevent open redirects on logout controller

* use clearer config key

* cast url as string, reinstate guest redirect

* clean up a little

* simplify

* return Uri

* resolve ternary always true

* simplify some more

* remove extra newline

* handle malformed uri

* chore: requested changes

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;
@@ -20,6 +21,7 @@ use Illuminate\Contracts\View\Factory;
 use Illuminate\Support\Arr;
 use Laminas\Diactoros\Response\HtmlResponse;
 use Laminas\Diactoros\Response\RedirectResponse;
+use Laminas\Diactoros\Uri;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Http\Server\RequestHandlerInterface;
@@ -51,25 +53,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 +91,14 @@ class LogOutController implements RequestHandlerInterface
     {
         $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());
+        $returnUrl = Arr::get($request->getQueryParams(), 'return');
+        $return = $this->sanitizeReturnUrl((string) $returnUrl, $base);
 
-        // If there is no user logged in, return to the index.
+        // If there is no user logged in, return to the index or the return url if it's set.
         if ($actor->isGuest()) {
-            return new RedirectResponse($url);
+            return new RedirectResponse($return);
         }
 
         // If a valid CSRF token hasn't been provided, show a view which will
@@ -94,16 +106,14 @@ class LogOutController implements RequestHandlerInterface
         $csrfToken = $session->token();
 
         if (Arr::get($request->getQueryParams(), 'token') !== $csrfToken) {
-            $return = Arr::get($request->getQueryParams(), 'return');
-
             $view = $this->view->make('flarum.forum::log-out')
-                ->with('url', $this->url->to('forum')->route('logout').'?token='.$csrfToken.($return ? '&return='.urlencode($return) : ''));
+                ->with('url', $this->url->to('forum')->route('logout') . '?token=' . $csrfToken . ($returnUrl ? '&return=' . urlencode($return) : ''));
 
             return new HtmlResponse($view->render());
         }
 
         $accessToken = $session->get('access_token');
-        $response = new RedirectResponse($url);
+        $response = new RedirectResponse($return);
 
         $this->authenticator->logOut($session);
 
@@ -113,4 +123,33 @@ class LogOutController implements RequestHandlerInterface
 
         return $this->rememberer->forget($response);
     }
+
+    protected function sanitizeReturnUrl(string $url, string $base): Uri
+    {
+        if (empty($url)) {
+            return new Uri($base);
+        }
+
+        try {
+            $parsedUrl = new Uri($url);
+        } catch (\InvalidArgumentException $e) {
+            return new Uri($base);
+        }
+
+        if (in_array($parsedUrl->getHost(), $this->getAllowedRedirectDomains())) {
+            return $parsedUrl;
+        }
+
+        return new Uri($base);
+    }
+
+    protected function getAllowedRedirectDomains(): array
+    {
+        $forumUri = $this->config->url();
+
+        return array_merge(
+            [$forumUri->getHost()],
+            $this->config->offsetGet('redirectDomains') ?? []
+        );
+    }
 }