Просмотр исходного кода

Rotate session ID on all authenticated transitions (CWE-384) (#9333)

* auth: rotate session ID on all authenticated transitions (session fixation)

The PHP session ID was regenerated on explicit form login, reauth, logout and
password change, but NOT when access is (re)established from a remember-me
cookie, via HTTP authentication, or on self-registration auto-login (CWE-384),
so a fixed session ID could become authenticated.

Regenerate the session ID at those transitions too: in FreshRSS_Auth::init
when accessControl() first grants access (covers remember-me restore and HTTP
auth; skipped for auth_type 'none', which has no authentication boundary), and
in the self-registration auto-login path, mirroring the existing form-login
idiom. Also enable session.use_strict_mode before session_start as defence in
depth so an uninitialised, attacker-supplied session ID is not adopted.

* auth: avoid duplicate cookie during session rotation

* Code preferences, shorten some comments

---------

Co-authored-by: Inverle <inverle@proton.me>
Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Gigi 1 день назад
Родитель
Сommit
5a270c02b0
3 измененных файлов с 42 добавлено и 3 удалено
  1. 7 0
      app/Controllers/userController.php
  2. 13 3
      app/Models/Auth.php
  3. 22 0
      lib/Minz/Session.php

+ 7 - 0
app/Controllers/userController.php

@@ -508,6 +508,13 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 			if ($ok && !FreshRSS_Auth::hasAccess('admin')) {
 				$user_conf = FreshRSS_UserConfiguration::getForUser($new_user_name);
 				if ($user_conf !== null) {
+					// Rotate the session ID on this unauthenticated->authenticated
+					// transition to prevent session fixation, as on form login.
+					try {
+						Minz_Session::regenerateID('FreshRSS');
+					} catch (RuntimeException $e) {
+						Minz_Log::error('Session could not be regenerated during self-registration auto-login! ' . $e->getMessage());
+					}
 					Minz_Session::_params([
 						Minz_User::CURRENT_USER => $new_user_name,
 						'passwordHash' => $user_conf->passwordHash,

+ 13 - 3
app/Models/Auth.php

@@ -34,9 +34,19 @@ class FreshRSS_Auth {
 		if (self::$login_ok && self::giveAccess()) {
 			return self::$login_ok;
 		}
-		if (self::accessControl() && self::giveAccess()) {
-			FreshRSS_UserDAO::touch();
-			return self::$login_ok;
+		if (self::accessControl()) {
+			// Rotate the PHP session ID on the unauthenticated->authenticated transition
+			if (FreshRSS_Context::systemConf()->auth_type !== 'none') {
+				try {
+					Minz_Session::regenerateID('FreshRSS');
+				} catch (RuntimeException $e) {
+					Minz_Log::error('Session could not be regenerated during access restoration: ' . $e->getMessage());
+				}
+			}
+			if (self::giveAccess()) {
+				FreshRSS_UserDAO::touch();
+				return self::$login_ok;
+			}
 		}
 		// Be sure all accesses are removed!
 		self::removeAccess();

+ 22 - 0
lib/Minz/Session.php

@@ -54,6 +54,9 @@ class Minz_Session {
 
 		session_name($name);
 
+		// Reject an uninitialized (e.g. attacker-supplied) session ID
+		ini_set('session.use_strict_mode', '1');
+
 		// When using cookies (default value), session_start() sends HTTP headers
 		session_start();
 		session_write_close();
@@ -234,6 +237,25 @@ class Minz_Session {
 		$params = session_get_cookie_params();
 		$params['expires'] = $params['lifetime'] > 0 ? time() + $params['lifetime'] : 0;
 		unset($params['lifetime']);
+
+		// session_start() may already have queued a cookie when there was no session
+		// cookie in the request (e.g. during remember-me auto-login).
+		$setCookieHeaders = [];
+		foreach (headers_list() as $header) {
+			if (stripos($header, 'Set-Cookie:') === 0) {
+				$setCookieHeaders[] = $header;
+			}
+		}
+		if ($setCookieHeaders !== []) {
+			header_remove('Set-Cookie');
+			$prefixLength = strlen('Set-Cookie:');
+			foreach ($setCookieHeaders as $header) {
+				$cookie = ltrim(substr($header, $prefixLength));
+				if (!str_starts_with($cookie, $name . '=')) {
+					header($header, replace: false);
+				}
+			}
+		}
 		if (!setcookie($name, $newId, $params)) {
 			throw new RuntimeException('Failed to set session cookie!');
 		}