CVE-2024-21641: URL Redirection to Untrusted Site ('Open Redirect') (CWE-601)
value: Flarum is open source discussion platform software. Prior to version 1.8.5, the Flarum `/logout` route includes a redirect parameter that allows any third party to redirect users from a (trusted) domain of the Flarum installation to redirect to any link. For logged-in users, the logout must be confirmed. Guests are immediately redirected. This could be used by spammers to redirect to a web address using a trusted domain of a running Flarum installation. The vulnerability has been fixed and published as flarum/core v1.8.5. As a workaround, some extensions modifying the logout route can remedy this issue if their implementation is safe.
CWE description: A web application accepts a user-controlled input that specifies a link to an external site, and uses that link in a Redirect. This simplifies phishing attacks.
CVSS v3 base score: 4.7
Published: 2024-01-05T21:15Z
Repository: https://github.com/flarum/flarum-core
Language: PHP
Files changed: src/Forum/Controller/LogOutController.php
Commit date: 2024-01-05 15:33:10 +0000
Commit message: [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
Diff: diff --git a/src/Forum/Controller/LogOutController.php b/src/Forum/Controller/LogOutController.php
index f5eb88d3..9f3ab2c0 100644
--- a/src/Forum/Controller/LogOutController.php
+++ b/src/Forum/Controller/LogOutController.php
@@ -7,10 +7,11 @@
  * LICENSE file that was distributed with this source code.
  */
 
 namespace Flarum\Forum\Controller;
 
+use Flarum\Foundation\Config;
 use Flarum\Http\Exception\TokenMismatchException;
 use Flarum\Http\Rememberer;
 use Flarum\Http\RequestUtil;
 use Flarum\Http\SessionAuthenticator;
 use Flarum\Http\UrlGenerator;
@@ -18,10 +19,11 @@ use Flarum\User\Event\LoggedOut;
 use Illuminate\Contracts\Events\Dispatcher;
 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;
 
 class LogOutController implements RequestHandlerInterface
@@ -49,29 +51,37 @@ class LogOutController implements RequestHandlerInterface
     /**
      * @var UrlGenerator
      */
     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;
     }
 
     /**
      * @param Request $request
      * @return ResponseInterface
@@ -79,38 +89,67 @@ class LogOutController implements RequestHandlerInterface
      */
     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());
+        $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
         // allow the user to press a button to complete the log out process.
         $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);
 
         $actor->accessTokens()->where('token', $accessToken)->delete();
 
         $this->events->dispatch(new LoggedOut($actor, false));
 
         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') ?? []
+        );
+    }
 }

Vulnerable code:         UrlGenerator $url
        $url = Arr::get($request->getQueryParams(), 'return', $this->url->to('forum')->base());
        // If there is no user logged in, return to the index.
            return new RedirectResponse($url);
            $return = Arr::get($request->getQueryParams(), 'return');

                ->with('url', $this->url->to('forum')->route('logout').'?token='.$csrfToken.($return ? '&return='.urlencode($return) : ''));
        $response = new RedirectResponse($url);
Fixed code: use Flarum\Foundation\Config;
use Laminas\Diactoros\Uri;
    /**
     * @var Config
     */
    protected $config;

     * @param Config $config
        UrlGenerator $url,
        Config $config
        $this->config = $config;
        $base = $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 or the return url if it's set.
            return new RedirectResponse($return);
                ->with('url', $this->url->to('forum')->route('logout') . '?token=' . $csrfToken . ($returnUrl ? '&return=' . urlencode($return) : ''));
        $response = new RedirectResponse($return);

    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') ?? []
        );
    }