فهرست منبع

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
Inverle 22 ساعت پیش
والد
کامیت
c4b96b253c
4فایلهای تغییر یافته به همراه6 افزوده شده و 11 حذف شده
  1. 1 0
      app/Controllers/authController.php
  2. 1 0
      app/Controllers/userController.php
  3. 2 9
      app/FreshRSS.php
  4. 2 2
      app/Models/Auth.php

+ 1 - 0
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'),

+ 1 - 0
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();

+ 2 - 9
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]']]);
 			}

+ 2 - 2
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,
 		]);