Merge branch 'develop' into v2

CHANGELOG.md

@@ -3,6 +3,10 @@ This file is a running track of new features and fixes to each version of the pa
 
 This project follows [Semantic Versioning](http://semver.org) guidelines.
 
+## v1.6.2
+### Fixed
+* **[Security]** Fixes an authentication bypass vulerability that could allow a malicious actor to login as another user in the Panel without knowing that user's email or password.
+
 ## v1.6.1
 ### Fixed
 * Fixes server build modifications not being properly persisted to the database when edited.

app/Http/Controllers/Auth/AbstractLoginController.php

@@ -7,7 +7,7 @@ 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;
@@ -17,6 +17,8 @@ abstract class AbstractLoginController extends Controller
 {
     use AuthenticatesUsers;
 
+    protected AuthManager $auth;
+
     /**
      * Lockout time for failed login requests.
      *
@@ -38,26 +40,14 @@ abstract class AbstractLoginController extends Controller
      */
     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->lockoutTime = config('auth.lockout.time');
-        $this->maxLoginAttempts = $config->get('auth.lockout.attempts');
+        $this->maxLoginAttempts = config('auth.lockout.attempts');
-
+        $this->auth = Container::getInstance()->make(AuthManager::class);
-        $this->auth = $auth;
-        $this->config = $config;
     }
 
     /**
@@ -72,7 +62,7 @@ abstract class AbstractLoginController extends Controller
             $this->getField($request->input('user')) => $request->input('user'),
         ]);
 
-        if ($request->route()->named('auth.login-checkpoint')) {
+        if ($request->route()->named('auth.checkpoint') || $request->route()->named('auth.checkpoint.key')) {
             throw new DisplayException($message ?? trans('auth.two_factor.checkpoint_failed'));
         }
 
@@ -84,7 +74,9 @@ abstract class AbstractLoginController extends Controller
      */
     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);
@@ -100,7 +92,8 @@ 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
     {

app/Http/Controllers/Auth/LoginCheckpointController.php

@@ -2,36 +2,35 @@
 
 namespace Pterodactyl\Http\Controllers\Auth;
 
+use Carbon\CarbonInterface;
+use Carbon\CarbonImmutable;
 use Pterodactyl\Models\User;
-use Illuminate\Auth\AuthManager;
 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 Illuminate\Contracts\Validation\Factory as ValidationFactory;
 
 class LoginCheckpointController extends AbstractLoginController
 {
-    private CacheRepository $cache;
+    public const TOKEN_EXPIRED_MESSAGE = 'The authentication token provided has expired, please refresh the page and try again.';
+
     private Encrypter $encrypter;
+
     private Google2FA $google2FA;
 
+    private ValidationFactory $validation;
+
     /**
      * LoginCheckpointController constructor.
      */
-    public function __construct(
+    public function __construct(Encrypter $encrypter, Google2FA $google2FA, ValidationFactory $validation)
-        AuthManager $auth,
+    {
-        Repository $config,
+        parent::__construct();
-        CacheRepository $cache,
-        Encrypter $encrypter,
-        Google2FA $google2FA
-    ) {
-        parent::__construct($auth, $config);
 
-        $this->cache = $cache;
         $this->encrypter = $encrypter;
         $this->google2FA = $google2FA;
+        $this->validation = $validation;
     }
 
     /**
@@ -45,28 +44,31 @@ class LoginCheckpointController extends AbstractLoginController
      * @throws \PragmaRX\Google2FA\Exceptions\InvalidCharactersException
      * @throws \PragmaRX\Google2FA\Exceptions\SecretKeyTooShortException
      * @throws \Illuminate\Validation\ValidationException
+     * @throws \Pterodactyl\Exceptions\DisplayException
      */
     public function __invoke(LoginCheckpointRequest $request)
     {
         if ($this->hasTooManyLoginAttempts($request)) {
             $this->sendLockoutResponse($request);
-
             return;
         }
 
-        $token = $request->input('confirmation_token');
+        $details = $request->session()->get('auth_confirmation_token');
+        if (!$this->hasValidSessionData($details)) {
+            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
+            return;
+        }
+
+        if (!hash_equals($request->input('confirmation_token') ?? '', $details['token_value'])) {
+            $this->sendFailedLoginResponse($request);
+            return;
+        }
+
         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);
+            $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE);
-
-            $this->sendFailedLoginResponse(
-                $request,
-                null,
-                'The authentication token provided has expired, please refresh the page and try again.'
-            );
-
             return;
         }
 
@@ -79,13 +81,10 @@ class LoginCheckpointController extends AbstractLoginController
             $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);
         $this->sendFailedLoginResponse($request, $user, !empty($recoveryToken) ? 'The recovery token provided is not valid.' : null);
     }
 
@@ -94,10 +93,8 @@ class LoginCheckpointController extends AbstractLoginController
      * it will be deleted from the database.
      *
      * @return bool
-     *
-     * @throws \Exception
      */
-    protected function isValidRecoveryToken(User $user, string $value)
+    protected function isValidRecoveryToken(User $user, string $value): bool
     {
         foreach ($user->recoveryTokens as $token) {
             if (password_verify($value, $token->token)) {
@@ -109,4 +106,40 @@ class LoginCheckpointController extends AbstractLoginController
 
         return false;
     }
+
+    protected function hasValidSessionData(array $data): bool
+    {
+        return static::isValidSessionData($this->validation, $data);
+    }
+
+    /**
+     * 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
+     */
+    public static function isValidSessionData(ValidationFactory $validation, array $data): bool
+    {
+        $validator = $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;
+    }
 }

app/Http/Controllers/Auth/LoginController.php

@@ -5,15 +5,12 @@ 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 LaravelWebauthn\Facades\Webauthn;
-use Illuminate\Contracts\Config\Repository;
 use Illuminate\Contracts\View\Factory as ViewFactory;
-use Illuminate\Contracts\Cache\Repository as CacheRepository;
+use Illuminate\Database\Eloquent\ModelNotFoundException;
-use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
-use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
 
 class LoginController extends AbstractLoginController
 {
@@ -22,31 +19,21 @@ class LoginController extends AbstractLoginController
     private const METHOD_TOTP = 'totp';
     private const METHOD_WEBAUTHN = 'webauthn';
 
-    private CacheRepository $cache;
-    private UserRepositoryInterface $repository;
     private ViewFactory $view;
 
     /**
      * LoginController constructor.
      */
-    public function __construct(
+    public function __construct(ViewFactory $view) {
-        AuthManager $auth,
+        parent::__construct();
-        Repository $config,
-        CacheRepository $cache,
-        UserRepositoryInterface $repository,
-        ViewFactory $view
-    ) {
-        parent::__construct($auth, $config);
 
-        $this->cache = $cache;
-        $this->repository = $repository;
         $this->view = $view;
     }
 
     /**
      * Handle all incoming requests for the authentication routes and render the
-     * base authentication view component.  React will take over at this point and
+     * base authentication view component. React will take over at this point and
-     * turn the login area into a SPA.
+     * turn the login area into an SPA.
      */
     public function index(): View
     {
@@ -63,9 +50,6 @@ class LoginController extends AbstractLoginController
      */
     public function login(Request $request)
     {
-        $username = $request->input('user');
-        $useColumn = $this->getField($username);
-
         if ($this->hasTooManyLoginAttempts($request)) {
             $this->fireLockoutEvent($request);
             $this->sendLockoutResponse($request);
@@ -74,12 +58,12 @@ class LoginController extends AbstractLoginController
         }
 
         try {
-            /** @var \Pterodactyl\Models\User $user */
+            $username = $request->input('user');
-            $user = $this->repository->findFirstWhere([[$useColumn, '=', $username]]);
-        } catch (RecordNotFoundException $exception) {
-            $this->sendFailedLoginResponse($request);
 
-            return;
+            /** @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
@@ -92,40 +76,44 @@ class LoginController extends AbstractLoginController
             return;
         }
 
+        $useTotp = $user->use_totp;
         $webauthnKeys = $user->webauthnKeys()->get();
 
-        if (count($webauthnKeys) > 0) {
+        if (!$useTotp && count($webauthnKeys) < 1) {
-            $token = Str::random(64);
+            return $this->sendLoginResponse($user, $request);
-            $this->cache->put($token, $user->id, CarbonImmutable::now()->addMinutes(5));
-
-            $publicKey = Webauthn::getAuthenticateData($user);
-            $request->session()->put(self::SESSION_PUBLICKEY_REQUEST, $publicKey);
-            $request->session()->save();
-
-            $methods = [self::METHOD_WEBAUTHN];
-            if ($user->use_totp) {
-                $methods[] = self::METHOD_TOTP;
-            }
-
-            return new JsonResponse([
-                'complete' => false,
-                'methods' => $methods,
-                'confirmation_token' => $token,
-                'webauthn' => [
-                    'public_key' => $publicKey,
-                ],
-            ]);
-        } elseif ($user->use_totp) {
-            $token = Str::random(64);
-            $this->cache->put($token, $user->id, CarbonImmutable::now()->addMinutes(5));
-
-            return new JsonResponse([
-                'complete' => false,
-                'methods' => [self::METHOD_TOTP],
-                'confirmation_token' => $token,
-            ]);
         }
 
-        return $this->sendLoginResponse($user, $request);
+        $methods = [];
+        if ($useTotp) {
+            $methods[] = self::METHOD_TOTP;
+        }
+        if (count($webauthnKeys) > 0) {
+            $methods[] = self::METHOD_WEBAUTHN;
+        }
+
+        $token = Str::random(64);
+
+        $request->session()->put('auth_confirmation_token', [
+            'user_id' => $user->id,
+            'token_value' => $token,
+            'expires_at' => CarbonImmutable::now()->addMinutes(5),
+        ]);
+
+        $response = [
+            'complete' => false,
+            'methods' => $methods,
+            'confirmation_token' => $token,
+        ];
+
+        if (count($webauthnKeys) > 0) {
+            $publicKey = Webauthn::getAuthenticateData($user);
+            $request->session()->put(self::SESSION_PUBLICKEY_REQUEST, $publicKey);
+
+            $response['webauthn'] = [
+                'public_key' => $publicKey,
+            ];
+        }
+
+        return new JsonResponse($response);
     }
 }

app/Http/Controllers/Auth/WebauthnController.php

@@ -5,13 +5,12 @@ namespace Pterodactyl\Http\Controllers\Auth;
 use Exception;
 use Illuminate\Http\Request;
 use Pterodactyl\Models\User;
-use Illuminate\Auth\AuthManager;
 use Illuminate\Http\JsonResponse;
 use LaravelWebauthn\Facades\Webauthn;
 use Webauthn\PublicKeyCredentialRequestOptions;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Contracts\Cache\Repository as CacheRepository;
-use Illuminate\Contracts\Config\Repository as ConfigRepository;
+use Illuminate\Contracts\Validation\Factory as ValidationFactory;
 
 class WebauthnController extends AbstractLoginController
 {
@@ -19,11 +18,14 @@ class WebauthnController extends AbstractLoginController
 
     private CacheRepository $cache;
 
-    public function __construct(AuthManager $auth, ConfigRepository $config, CacheRepository $cache)
+    private ValidationFactory $validation;
+
+    public function __construct(CacheRepository $cache, ValidationFactory $validation)
     {
-        parent::__construct($auth, $config);
+        parent::__construct();
 
         $this->cache = $cache;
+        $this->validation = $validation;
     }
 
     /**
@@ -36,25 +38,28 @@ class WebauthnController extends AbstractLoginController
     {
         if ($this->hasTooManyLoginAttempts($request)) {
             $this->sendLockoutResponse($request);
-
             return;
         }
 
-        $token = $request->input('confirmation_token');
+        $details = $request->session()->get('auth_confirmation_token');
+        if (!LoginCheckpointController::isValidSessionData($this->validation, $details)) {
+            $this->sendFailedLoginResponse($request, null, LoginCheckpointController::TOKEN_EXPIRED_MESSAGE);
+            return;
+        }
+
+        if (!hash_equals($request->input('confirmation_token') ?? '', $details['token_value'])) {
+            $this->sendFailedLoginResponse($request);
+            return;
+        }
+
         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);
+            $this->sendFailedLoginResponse($request, null, LoginCheckpointController::TOKEN_EXPIRED_MESSAGE);
-
-            $this->sendFailedLoginResponse(
-                $request,
-                null,
-                'The authentication token provided has expired, please refresh the page and try again.'
-            );
-
             return;
         }
+
         $this->auth->guard()->onceUsingId($user->id);
 
         try {
@@ -77,8 +82,6 @@ class WebauthnController extends AbstractLoginController
                 ], JsonResponse::HTTP_I_AM_A_TEAPOT);
             }
 
-            $this->cache->delete($token);
-
             return $this->sendLoginResponse($user, $request);
         } catch (Exception $e) {
             return new JsonResponse([

routes/auth.php

@@ -19,8 +19,8 @@ Route::group(['middleware' => 'guest'], function () {
 
     // Login endpoints.
     Route::post('/login', 'LoginController@login')->middleware('recaptcha');
-    Route::post('/login/checkpoint', 'LoginCheckpointController')->name('auth.login-checkpoint');
+    Route::post('/login/checkpoint', 'LoginCheckpointController')->name('auth.checkpoint');
-    Route::post('/login/checkpoint/key', 'WebauthnController@auth');
+    Route::post('/login/checkpoint/key', 'WebauthnController@auth')->name('auth.checkpoint.key');
 
     // Forgot password route. A post to this endpoint will trigger an
     // email to be sent containing a reset token.