From c4b96b253c59e61df72c7f792288ce893e5d85c8 Mon Sep 17 00:00:00 2001 From: Inverle Date: Tue, 8 Sep 2026 09:19:01 +0200 Subject: [PATCH] Fix auth CSRFs (login and register actions) (#9171) This is done by getting rid of the CSRF check allowlist, and instead of refreshing the CSRF token on every unauthenticated page load, only refreshing the token when needed (on login, logout, account delete, HTTP auth user change) In addition, the actualize actions are now correctly protected as well. The most notable part is that the user create action could potentially be exploited by another website for XSS on a given instance, by submitting the register form with known credentials, and then by the attacker logging into the same account forcibly registered by a malicious website, and setting custom User JS code. Login action was likely not exploitable in any meaningful way, because of the JS nonce. It's worth noting one risk of merging this PR, some vulnerabilities discovered in the past had reduced impact due to CSRF token refreshing on every page load: https://github.com/FreshRSS/FreshRSS/security/advisories/GHSA-jf4v-f8p2-8xvq#:~:text=It%27s%20important,FreshRSS%5FAuth%3A%3AremoveAccess%28%29%2E --- app/Controllers/authController.php | 1 + app/Controllers/userController.php | 1 + app/FreshRSS.php | 11 ++--------- app/Models/Auth.php | 4 ++-- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/app/Controllers/authController.php b/app/Controllers/authController.php index d279d15bd..e5afda72b 100644 --- a/app/Controllers/authController.php +++ b/app/Controllers/authController.php @@ -230,6 +230,7 @@ class FreshRSS_auth_Controller extends FreshRSS_ActionController { if (Minz_Request::isPost()) { invalidateHttpCache(); FreshRSS_Auth::removeAccess(); + Minz_Session::_param('csrf', false); Minz_Session::regenerateID('FreshRSS'); Minz_Request::good( _t('feedback.auth.logout.success'), diff --git a/app/Controllers/userController.php b/app/Controllers/userController.php index 8b356553f..c0a75c95a 100644 --- a/app/Controllers/userController.php +++ b/app/Controllers/userController.php @@ -713,6 +713,7 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController { if ($ok && $self_deletion) { FreshRSS_Auth::removeAccess(); + Minz_Session::_param('csrf', false); $redirect_url = ['c' => 'index', 'a' => 'index']; } invalidateHttpCache(); diff --git a/app/FreshRSS.php b/app/FreshRSS.php index 251425de3..1376285e8 100644 --- a/app/FreshRSS.php +++ b/app/FreshRSS.php @@ -72,15 +72,8 @@ class FreshRSS extends Minz_FrontController { private static function initAuth(): void { FreshRSS_Auth::init(); if (Minz_Request::isPost()) { - if (!FreshRSS_Context::hasSystemConf() || !(FreshRSS_Auth::isCsrfOk() || - (Minz_Request::controllerName() === 'auth' && Minz_Request::actionName() === 'login') || - (Minz_Request::controllerName() === 'user' && Minz_Request::actionName() === 'create' && !FreshRSS_Auth::hasAccess('admin')) || - (Minz_Request::controllerName() === 'feed' && Minz_Request::actionName() === 'actualize' && - FreshRSS_Auth::allowAnonymousRefresh()) || - (Minz_Request::controllerName() === 'javascript' && Minz_Request::actionName() === 'actualize' && - FreshRSS_Auth::allowAnonymous()) - )) { - // Token-based protection against XSRF attacks, except for the login or self-create user forms + if (!FreshRSS_Context::hasSystemConf() || !FreshRSS_Auth::isCsrfOk()) { + // Token-based protection against CSRF attacks self::initI18n(); Minz_Error::error(403, ['error' => [_t('feedback.access.denied'), ' [CSRF]']]); } diff --git a/app/Models/Auth.php b/app/Models/Auth.php index dd925b676..10883057d 100644 --- a/app/Models/Auth.php +++ b/app/Models/Auth.php @@ -19,6 +19,7 @@ class FreshRSS_Auth { if (isset($_SESSION['REMOTE_USER']) && $_SESSION['REMOTE_USER'] !== FreshRSS_http_Util::httpAuthUser()) { //HTTP REMOTE_USER has changed self::removeAccess(); + Minz_Session::_param('csrf', false); } self::$login_ok = Minz_Session::paramBoolean('loginOk'); @@ -27,7 +28,6 @@ class FreshRSS_Auth { $current_user = FreshRSS_Context::systemConf()->default_user; Minz_Session::_params([ Minz_User::CURRENT_USER => $current_user, - 'csrf' => false, ]); } @@ -166,7 +166,7 @@ class FreshRSS_Auth { Minz_Session::_params([ 'loginOk' => false, 'lastReauth' => false, - 'csrf' => false, + // 'csrf' => false, // Must be refreshed separately 'REMOTE_USER' => false, ]);