diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b3da3007..f72f306f5 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/app/Http/Controllers/Auth/AbstractLoginController.php b/app/Http/Controllers/Auth/AbstractLoginController.php index b490d6036..2aced9b0c 100644 --- a/app/Http/Controllers/Auth/AbstractLoginController.php +++ b/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->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); } /** @@ -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 { diff --git a/app/Http/Controllers/Auth/LoginCheckpointController.php b/app/Http/Controllers/Auth/LoginCheckpointController.php index d38f31230..b9265884b 100644 --- a/app/Http/Controllers/Auth/LoginCheckpointController.php +++ b/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( - AuthManager $auth, - Repository $config, - CacheRepository $cache, - Encrypter $encrypter, - Google2FA $google2FA - ) { - parent::__construct($auth, $config); + public function __construct(Encrypter $encrypter, Google2FA $google2FA, ValidationFactory $validation) + { + parent::__construct(); - $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, - 'The authentication token provided has expired, please refresh the page and try again.' - ); - + $this->sendFailedLoginResponse($request, null, self::TOKEN_EXPIRED_MESSAGE); 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; + } } diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index bf12ceb82..d540469e1 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/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 Pterodactyl\Contracts\Repository\UserRepositoryInterface; -use Pterodactyl\Exceptions\Repository\RecordNotFoundException; +use Illuminate\Database\Eloquent\ModelNotFoundException; 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( - AuthManager $auth, - Repository $config, - CacheRepository $cache, - UserRepositoryInterface $repository, - ViewFactory $view - ) { - parent::__construct($auth, $config); + public function __construct(ViewFactory $view) { + parent::__construct(); - $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 - * turn the login area into a SPA. + * base authentication view component. React will take over at this point and + * 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 */ - $user = $this->repository->findFirstWhere([[$useColumn, '=', $username]]); - } catch (RecordNotFoundException $exception) { - $this->sendFailedLoginResponse($request); + $username = $request->input('user'); - 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) { - $token = Str::random(64); - $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, - ]); + if (!$useTotp && count($webauthnKeys) < 1) { + return $this->sendLoginResponse($user, $request); } - 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); } } diff --git a/app/Http/Controllers/Auth/WebauthnController.php b/app/Http/Controllers/Auth/WebauthnController.php index 7a85b9e19..42c104d14 100644 --- a/app/Http/Controllers/Auth/WebauthnController.php +++ b/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, - 'The authentication token provided has expired, please refresh the page and try again.' - ); - + $this->sendFailedLoginResponse($request, null, LoginCheckpointController::TOKEN_EXPIRED_MESSAGE); 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([ diff --git a/routes/auth.php b/routes/auth.php index e7fe3773c..75eb75931 100644 --- a/routes/auth.php +++ b/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/key', 'WebauthnController@auth'); + Route::post('/login/checkpoint', 'LoginCheckpointController')->name('auth.checkpoint'); + 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.