CVE-2021-41129: Improper Authentication (CWE-287)
value: Pterodactyl is an open-source game server management panel built with PHP 7, React, and Go. A malicious user can modify the contents of a `confirmation_token` input during the two-factor authentication process to reference a cache value not associated with the login attempt. In rare cases this can allow a malicious actor to authenticate as a random user in the Panel. The malicious user must target an account with two-factor authentication enabled, and then must provide a correct two-factor authentication token before being authenticated as that user. Due to a validation flaw in the logic handling user authentication during the two-factor authentication process a malicious user can trick the system into loading credentials for an arbitrary user by modifying the token sent to the server. This authentication flaw is present in the `LoginCheckpointController@__invoke` method which handles two-factor authentication for a user. This controller looks for a request input parameter called `confirmation_token` which is expected to be a 64 character random alpha-numeric string that references a value within the Panel's cache containing a `user_id` value. This value is then used to fetch the user that attempted to login, and lookup their two-factor authentication token. Due to the design of this system, any element in the cache that contains only digits could be referenced by a malicious user, and whatever value is stored at that position would be used as the `user_id`. There are a few different areas of the Panel that store values into the cache that are integers, and a user who determines what those cache keys are could pass one of those keys which would cause this code pathway to reference an arbitrary user. At its heart this is a high-risk login bypass vulnerability. However, there are a few additional conditions that must be met in order for this to be successfully executed, notably: 1.) The account referenced by the malicious cache key must have two-factor authentication enabled. An account without two-factor authentication would cause an exception to be triggered by the authentication logic, thusly exiting this authentication flow. 2.) Even if the malicious user is able to reference a valid cache key that references a valid user account with two-factor authentication, they must provide a valid two-factor authentication token. However, due to the design of this endpoint once a valid user account is found with two-factor authentication enabled there is no rate-limiting present, thusly allowing an attacker to brute force combinations until successful. This leads to a third condition that must be met: 3.) For the duration of this attack sequence the cache key being referenced must continue to exist with a valid `user_id` value. Depending on the specific key being used for this attack, this value may disappear quickly, or be changed by other random user interactions on the Panel, outside the control of the attacker. In order to mitigate this vulnerability the underlying authentication logic was changed to use an encrypted session store that the user is therefore unable to control the value of. This completely removed the use of a user-controlled value being used. In addition, the code was audited to ensure this type of vulnerability is not present elsewhere.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 8.1
Published: 2021-10-06T20:15Z
Repository: https://github.com/pterodactyl/panel
Language: PHP
Files changed: app/Http/Controllers/Auth/AbstractLoginController.php, app/Http/Controllers/Auth/LoginCheckpointController.php, app/Http/Controllers/Auth/LoginController.php
Commit date: 2021-09-21 21:30:08 -0700
Commit message: Fix security vulnerability when authenticating a two-factor authentication token for a user

See associated security advisory for technical details on the content of this security fix.

GHSA ID: GHSA-5vfx-8w6m-h3v4

Diff: diff --git a/app/Http/Controllers/Auth/AbstractLoginController.php b/app/Http/Controllers/Auth/AbstractLoginController.php
index 601cdc14..e50a8050 100644
--- a/app/Http/Controllers/Auth/AbstractLoginController.php
+++ b/app/Http/Controllers/Auth/AbstractLoginController.php
@@ -5,20 +5,22 @@ namespace Pterodactyl\Http\Controllers\Auth;
 use Illuminate\Http\Request;
 use Pterodactyl\Models\User;
 use Illuminate\Auth\AuthManager;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Auth\Events\Failed;
-use Illuminate\Contracts\Config\Repository;
+use Illuminate\Container\Container;
 use Pterodactyl\Exceptions\DisplayException;
 use Pterodactyl\Http\Controllers\Controller;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Foundation\Auth\AuthenticatesUsers;
 
 abstract class AbstractLoginController extends Controller
 {
     use AuthenticatesUsers;
 
+    protected AuthManager $auth;
+
     /**
      * Lockout time for failed login requests.
      *
      * @var int
      */
@@ -36,30 +38,18 @@ abstract class AbstractLoginController extends Controller
      *
      * @var string
      */
     protected $redirectTo = '/';
 
-    /**
-     * @var \Illuminate\Auth\AuthManager
-     */
-    protected $auth;
-
-    /**
-     * @var \Illuminate\Contracts\Config\Repository
-     */
-    protected $config;
-
     /**
      * LoginController constructor.
      */
-    public function __construct(AuthManager $auth, Repository $config)
+    public function __construct()
     {
-        $this->lockoutTime = $config->get('auth.lockout.time');
-        $this->maxLoginAttempts = $config->get('auth.lockout.attempts');
-
-        $this->auth = $auth;
-        $this->config = $config;
+        $this->lockoutTime = config('auth.lockout.time');
+        $this->maxLoginAttempts = config('auth.lockout.attempts');
+        $this->auth = Container::getInstance()->make(AuthManager::class);
     }
 
     /**
      * Get the failed login response instance.
      *
@@ -82,16 +72,18 @@ abstract class AbstractLoginController extends Controller
     /**
      * Send the response after the user was authenticated.
      */
     protected function sendLoginResponse(User $user, Request $request): JsonResponse
     {
+        $request->session()->remove('auth_confirmation_token');
         $request->session()->regenerate();
+
         $this->clearLoginAttempts($request);
 
         $this->auth->guard()->login($user, true);
 
-        return JsonResponse::create([
+        return new JsonResponse([
             'data' => [
                 'complete' => true,
                 'intended' => $this->redirectPath(),
                 'user' => $user->toVueObject(),
             ],
@@ -99,11 +91,12 @@ abstract class AbstractLoginController extends Controller
     }
 
     /**
      * Determine if the user is logging in using an email or username,.
      *
-     * @param string $input
+     * @param string|null $input
+     * @return string
      */
     protected function getField(string $input = null): string
     {
         return ($input && str_contains($input, '@')) ? 'email' : 'username';
     }
diff --git a/app/Http/Controllers/Auth/LoginCheckpointController.php b/app/Http/Controllers/Auth/LoginCheckpointController.php
index 3710da3a..fdb5b7e1 100644
--- a/app/Http/Controllers/Auth/LoginCheckpointController.php
+++ b/app/Http/Controllers/Auth/LoginCheckpointController.php
@@ -1,67 +1,39 @@
 <?php
 
 namespace Pterodactyl\Http\Controllers\Auth;
 
+use Carbon\CarbonInterface;
+use Carbon\CarbonImmutable;
 use Pterodactyl\Models\User;
-use Illuminate\Auth\AuthManager;
 use Illuminate\Http\JsonResponse;
 use PragmaRX\Google2FA\Google2FA;
-use Illuminate\Contracts\Config\Repository;
 use Illuminate\Contracts\Encryption\Encrypter;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Pterodactyl\Http\Requests\Auth\LoginCheckpointRequest;
-use Illuminate\Contracts\Cache\Repository as CacheRepository;
-use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
-use Pterodactyl\Repositories\Eloquent\RecoveryTokenRepository;
+use Illuminate\Contracts\Validation\Factory as ValidationFactory;
 
 class LoginCheckpointController extends AbstractLoginController
 {
-    /**
-     * @var \Illuminate\Contracts\Cache\Repository
-     */
-    private $cache;
-
-    /**
-     * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
-     */
-    private $repository;
+    private const TOKEN_EXPIRED_MESSAGE = 'The authentication token provided has expired, please refresh the page and try again.';
 
-    /**
-     * @var \PragmaRX\Google2FA\Google2FA
-     */
-    private $google2FA;
+    private ValidationFactory $validation;
 
-    /**
-     * @var \Illuminate\Contracts\Encryption\Encrypter
-     */
-    private $encrypter;
+    private Google2FA $google2FA;
 
-    /**
-     * @var \Pterodactyl\Repositories\Eloquent\RecoveryTokenRepository
-     */
-    private $recoveryTokenRepository;
+    private Encrypter $encrypter;
 
     /**
      * LoginCheckpointController constructor.
      */
-    public function __construct(
-        AuthManager $auth,
-        Encrypter $encrypter,
-        Google2FA $google2FA,
-        Repository $config,
-        CacheRepository $cache,
-        RecoveryTokenRepository $recoveryTokenRepository,
-        UserRepositoryInterface $repository
-    ) {
-        parent::__construct($auth, $config);
+    public function __construct(Encrypter $encrypter, Google2FA $google2FA, ValidationFactory $validation)
+    {
+        parent::__construct();
 
         $this->google2FA = $google2FA;
-        $this->cache = $cache;
-        $this->repository = $repository;
         $this->encrypter = $encrypter;
-        $this->recoveryTokenRepository = $recoveryTokenRepository;
+        $this->validation = $validation;
     }
 
     /**
      * Handle a login where the user is required to provide a TOTP authentication
      * token. Once a user has reached this stage it is assumed that they have already
@@ -79,22 +51,24 @@ class LoginCheckpointController extends AbstractLoginController
     {
         if ($this->hasTooManyLoginAttempts($request)) {
             $this->sendLockoutResponse($request);
         }
 
-        $token = $request->input('confirmation_token');
+        $details = $request->session()->get('auth_confirmation_token');
+        if (!$this->hasValidSessionData($details)) {
+            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
+        }
+
+        if (!hash_equals($request->input('confirmation_token') ?? '', $details['token_value'])) {
+            $this->sendFailedLoginResponse($request);
+        }
+
         try {
             /** @var \Pterodactyl\Models\User $user */
-            $user = User::query()->findOrFail($this->cache->get($token, 0));
+            $user = User::query()->findOrFail($details['user_id']);
         } catch (ModelNotFoundException $exception) {
-            $this->incrementLoginAttempts($request);
-
-            return $this->sendFailedLoginResponse(
-                $request,
-                null,
-                'The authentication token provided has expired, please refresh the page and try again.'
-            );
+            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
         }
 
         // Recovery tokens go through a slightly different pathway for usage.
         if (!is_null($recoveryToken = $request->input('recovery_token'))) {
             if ($this->isValidRecoveryToken($user, $recoveryToken)) {
@@ -102,19 +76,15 @@ class LoginCheckpointController extends AbstractLoginController
             }
         } else {
             $decrypted = $this->encrypter->decrypt($user->totp_secret);
 
             if ($this->google2FA->verifyKey($decrypted, (string) $request->input('authentication_code') ?? '', config('pterodactyl.auth.2fa.window'))) {
-                $this->cache->delete($token);
-
                 return $this->sendLoginResponse($user, $request);
             }
         }
 
-        $this->incrementLoginAttempts($request);
-
-        return $this->sendFailedLoginResponse($request, $user, !empty($recoveryToken) ? 'The recovery token provided is not valid.' : null);
+        $this->sendFailedLoginResponse($request, $user, !empty($recoveryToken) ? 'The recovery token provided is not valid.' : null);
     }
 
     /**
      * Determines if a given recovery token is valid for the user account. If we find a matching token
      * it will be deleted from the database.
@@ -133,6 +103,37 @@ class LoginCheckpointController extends AbstractLoginController
             }
         }
 
         return false;
     }
+
+    /**
+     * Determines if the data provided from the session is valid or not. This
+     * will return false if the data is invalid, or if more time has passed than
+     * was configured when the session was written.
+     *
+     * @param array $data
+     * @return bool
+     */
+    protected function hasValidSessionData(array $data): bool
+    {
+        $validator = $this->validation->make($data, [
+            'user_id' => 'required|integer|min:1',
+            'token_value' => 'required|string',
+            'expires_at' => 'required',
+        ]);
+
+        if ($validator->fails()) {
+            return false;
+        }
+
+        if (!$data['expires_at'] instanceof CarbonInterface) {
+            return false;
+        }
+
+        if ($data['expires_at']->isBefore(CarbonImmutable::now())) {
+            return false;
+        }
+
+        return true;
+    }
 }
diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php
index db74ab12..3c477c55 100644
--- a/app/Http/Controllers/Auth/LoginController.php
+++ b/app/Http/Controllers/Auth/LoginController.php
@@ -3,51 +3,28 @@
 namespace Pterodactyl\Http\Controllers\Auth;
 
 use Carbon\CarbonImmutable;
 use Illuminate\Support\Str;
 use Illuminate\Http\Request;
-use Illuminate\Auth\AuthManager;
+use Pterodactyl\Models\User;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Contracts\View\View;
-use Illuminate\Contracts\Config\Repository;
 use Illuminate\Contracts\View\Factory as ViewFactory;
-use Illuminate\Contracts\Cache\Repository as CacheRepository;
-use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
-use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
 
 class LoginController extends AbstractLoginController
 {
-    /**
-     * @var \Illuminate\Contracts\View\Factory
-     */
-    private $view;
-
-    /**
-     * @var \Illuminate\Contracts\Cache\Repository
-     */
-    private $cache;
-
-    /**
-     * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
-     */
-    private $repository;
+    private ViewFactory $view;
 
     /**
      * LoginController constructor.
      */
-    public function __construct(
-        AuthManager $auth,
-        Repository $config,
-        CacheRepository $cache,
-        UserRepositoryInterface $repository,
-        ViewFactory $view
-    ) {
-        parent::__construct($auth, $config);
+    public function __construct(ViewFactory $view)
+    {
+        parent::__construct();
 
         $this->view = $view;
-        $this->cache = $cache;
-        $this->repository = $repository;
     }
 
     /**
      * Handle all incoming requests for the authentication routes and render the
      * base authentication view component. Vuejs will take over at this point and
@@ -66,35 +43,40 @@ class LoginController extends AbstractLoginController
      * @throws \Pterodactyl\Exceptions\DisplayException
      * @throws \Illuminate\Validation\ValidationException
      */
     public function login(Request $request): JsonResponse
     {
-        $username = $request->input('user');
-        $useColumn = $this->getField($username);
-
         if ($this->hasTooManyLoginAttempts($request)) {
             $this->fireLockoutEvent($request);
             $this->sendLockoutResponse($request);
         }
 
         try {
-            $user = $this->repository->findFirstWhere([[$useColumn, '=', $username]]);
-        } catch (RecordNotFoundException $exception) {
-            return $this->sendFailedLoginResponse($request);
+            $username = $request->input('user');
+
+            /** @var \Pterodactyl\Models\User $user */
+            $user = User::query()->where($this->getField($username), $username)->firstOrFail();
+        } catch (ModelNotFoundException $exception) {
+            $this->sendFailedLoginResponse($request);
         }
 
         // Ensure that the account is using a valid username and password before trying to
         // continue. Previously this was handled in the 2FA checkpoint, however that has
         // a flaw in which you can discover if an account exists simply by seeing if you
         // can proceede to the next step in the login process.
         if (!password_verify($request->input('password'), $user->password)) {
-            return $this->sendFailedLoginResponse($request, $user);
+            $this->sendFailedLoginResponse($request, $user);
         }
 
         if ($user->use_totp) {
             $token = Str::random(64);
-            $this->cache->put($token, $user->id, CarbonImmutable::now()->addMinutes(5));
+
+            $request->session()->put('auth_confirmation_token', [
+                'user_id' => $user->id,
+                'token_value' => $token,
+                'expires_at' => CarbonImmutable::now()->addMinutes(5),
+            ]);
 
             return new JsonResponse([
                 'data' => [
                     'complete' => false,
                     'confirmation_token' => $token,

Vulnerable code: use Illuminate\Contracts\Config\Repository;
    /**
     * @var \Illuminate\Auth\AuthManager
     */
    protected $auth;

    /**
     * @var \Illuminate\Contracts\Config\Repository
     */
    protected $config;

    public function __construct(AuthManager $auth, Repository $config)
        $this->lockoutTime = $config->get('auth.lockout.time');
        $this->maxLoginAttempts = $config->get('auth.lockout.attempts');

        $this->auth = $auth;
        $this->config = $config;
        return JsonResponse::create([
     * @param string $input
use Illuminate\Auth\AuthManager;
use Illuminate\Contracts\Config\Repository;
use Illuminate\Contracts\Cache\Repository as CacheRepository;
use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
use Pterodactyl\Repositories\Eloquent\RecoveryTokenRepository;
    /**
     * @var \Illuminate\Contracts\Cache\Repository
     */
    private $cache;

    /**
     * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
     */
    private $repository;
    /**
     * @var \PragmaRX\Google2FA\Google2FA
     */
    private $google2FA;
    /**
     * @var \Illuminate\Contracts\Encryption\Encrypter
     */
    private $encrypter;
    /**
     * @var \Pterodactyl\Repositories\Eloquent\RecoveryTokenRepository
     */
    private $recoveryTokenRepository;
    public function __construct(
        AuthManager $auth,
        Encrypter $encrypter,
        Google2FA $google2FA,
        Repository $config,
        CacheRepository $cache,
        RecoveryTokenRepository $recoveryTokenRepository,
        UserRepositoryInterface $repository
    ) {
        parent::__construct($auth, $config);
        $this->cache = $cache;
        $this->repository = $repository;
        $this->recoveryTokenRepository = $recoveryTokenRepository;
        $token = $request->input('confirmation_token');
            $user = User::query()->findOrFail($this->cache->get($token, 0));
            $this->incrementLoginAttempts($request);

            return $this->sendFailedLoginResponse(
                $request,
                null,
                'The authentication token provided has expired, please refresh the page and try again.'
            );
                $this->cache->delete($token);

        $this->incrementLoginAttempts($request);

        return $this->sendFailedLoginResponse($request, $user, !empty($recoveryToken) ? 'The recovery token provided is not valid.' : null);
use Illuminate\Auth\AuthManager;
use Illuminate\Contracts\Config\Repository;
use Illuminate\Contracts\Cache\Repository as CacheRepository;
use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
    /**
     * @var \Illuminate\Contracts\View\Factory
     */
    private $view;

    /**
     * @var \Illuminate\Contracts\Cache\Repository
     */
    private $cache;

    /**
     * @var \Pterodactyl\Contracts\Repository\UserRepositoryInterface
     */
    private $repository;
    public function __construct(
        AuthManager $auth,
        Repository $config,
        CacheRepository $cache,
        UserRepositoryInterface $repository,
        ViewFactory $view
    ) {
        parent::__construct($auth, $config);
        $this->cache = $cache;
        $this->repository = $repository;
        $username = $request->input('user');
        $useColumn = $this->getField($username);

            $user = $this->repository->findFirstWhere([[$useColumn, '=', $username]]);
        } catch (RecordNotFoundException $exception) {
            return $this->sendFailedLoginResponse($request);
            return $this->sendFailedLoginResponse($request, $user);
            $this->cache->put($token, $user->id, CarbonImmutable::now()->addMinutes(5));
Fixed code: use Illuminate\Container\Container;
    protected AuthManager $auth;

    public function __construct()
        $this->lockoutTime = config('auth.lockout.time');
        $this->maxLoginAttempts = config('auth.lockout.attempts');
        $this->auth = Container::getInstance()->make(AuthManager::class);
        $request->session()->remove('auth_confirmation_token');

        return new JsonResponse([
     * @param string|null $input
     * @return string
use Carbon\CarbonInterface;
use Carbon\CarbonImmutable;
use Illuminate\Contracts\Validation\Factory as ValidationFactory;
    private const TOKEN_EXPIRED_MESSAGE = 'The authentication token provided has expired, please refresh the page and try again.';
    private ValidationFactory $validation;
    private Google2FA $google2FA;
    private Encrypter $encrypter;
    public function __construct(Encrypter $encrypter, Google2FA $google2FA, ValidationFactory $validation)
    {
        parent::__construct();
        $this->validation = $validation;
        $details = $request->session()->get('auth_confirmation_token');
        if (!$this->hasValidSessionData($details)) {
            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
        }

        if (!hash_equals($request->input('confirmation_token') ?? '', $details['token_value'])) {
            $this->sendFailedLoginResponse($request);
        }

            $user = User::query()->findOrFail($details['user_id']);
            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
        $this->sendFailedLoginResponse($request, $user, !empty($recoveryToken) ? 'The recovery token provided is not valid.' : null);

    /**
     * Determines if the data provided from the session is valid or not. This
     * will return false if the data is invalid, or if more time has passed than
     * was configured when the session was written.
     *
     * @param array $data
     * @return bool
     */
    protected function hasValidSessionData(array $data): bool
    {
        $validator = $this->validation->make($data, [
            'user_id' => 'required|integer|min:1',
            'token_value' => 'required|string',
            'expires_at' => 'required',
        ]);

        if ($validator->fails()) {
            return false;
        }

        if (!$data['expires_at'] instanceof CarbonInterface) {
            return false;
        }

        if ($data['expires_at']->isBefore(CarbonImmutable::now())) {
            return false;
        }

        return true;
    }
use Pterodactyl\Models\User;
use Illuminate\Database\Eloquent\ModelNotFoundException;
    private ViewFactory $view;
    public function __construct(ViewFactory $view)
    {
        parent::__construct();
            $username = $request->input('user');

            /** @var \Pterodactyl\Models\User $user */
            $user = User::query()->where($this->getField($username), $username)->firstOrFail();
        } catch (ModelNotFoundException $exception) {
            $this->sendFailedLoginResponse($request);
            $this->sendFailedLoginResponse($request, $user);

            $request->session()->put('auth_confirmation_token', [
                'user_id' => $user->id,
                'token_value' => $token,
                'expires_at' => CarbonImmutable::now()->addMinutes(5),
            ]);