CVE-2023-38489: Insufficient Session Expiration (CWE-613)
value: Kirby is a content management system. A vulnerability in versions prior to 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1, and 3.9.6 affects all Kirby sites with user accounts (unless Kirby's API and Panel are disabled in the config). It can only be abused if a Kirby user is logged in on a device or browser that is shared with potentially untrusted users or if an attacker already maliciously used a previous password to log in to a Kirby site as the affected user.

Insufficient Session Expiration is when a web site permits an attacker to reuse old session credentials or session IDs for authorization. In the variation described in this advisory, it allows attackers to stay logged in to a Kirby site on another device even if the logged in user has since changed their password. Kirby did not invalidate user sessions that were created with a password that was since changed by the user or by a site admin. If a user changed their password to lock out an attacker who was already in possession of the previous password or of a login session on another device or browser, the attacker would not be reliably prevented from accessing the Kirby site as the affected user.

The problem has been patched in Kirby 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1, and 3.9.6. In all of the mentioned releases, the maintainers have updated the authentication implementation to keep track of the hashed password in each active session. If the password changed since the login, the session is invalidated. To enforce this fix even if the vulnerability was previously abused, all users are logged out from the Kirby site after updating to one of the patched releases.
CWE description: According to WASC, "Insufficient Session Expiration is when a web site permits an attacker to reuse old session credentials or session IDs for authorization."
CVSS v3 base score: 7.3
Published: 2023-07-27T15:15Z
Repository: https://github.com/getkirby/kirby
Language: PHP
Files changed: src/Cms/Auth.php, src/Cms/User.php, src/Cms/UserActions.php, tests/Cms/Auth/AuthTest.php, tests/Cms/Users/UserActionsTest.php, tests/Cms/Users/UserAuthTest.php, tests/Cms/Users/UserTest.php
Commit date: 2023-07-26 21:17:35 +0200
Commit message: Invalidate session on password change

https://github.com/getkirby/kirby/security/advisories/GHSA-5mvj-rvp8-rf45

Diff: diff --git a/src/Cms/Auth.php b/src/Cms/Auth.php
index 34fc07328..6f9ed827d 100644
--- a/src/Cms/Auth.php
+++ b/src/Cms/Auth.php
@@ -277,22 +277,43 @@ class Auth
 	{
 		$session = $this->session($session);
 
 		$id = $session->data()->get('kirby.userId');
 
+		// if no user is logged in, return immediately
 		if (is_string($id) !== true) {
 			return null;
 		}
 
-		if ($user = $this->kirby->users()->find($id)) {
-			// in case the session needs to be updated, do it now
-			// for better performance
-			$session->commit();
-			return $user;
+		// a user is logged in, ensure it exists
+		$user = $this->kirby->users()->find($id);
+		if ($user === null) {
+			return null;
 		}
 
-		return null;
+		if ($passwordTimestamp = $user->passwordTimestamp()) {
+			$loginTimestamp = $session->data()->get('kirby.loginTimestamp');
+			if (is_int($loginTimestamp) !== true) {
+				// session that was created before Kirby
+				// 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1 or 3.9.6
+				// or when the user didn't have a password set
+				$user->logout();
+				return null;
+			}
+
+			// invalidate the session if the password
+			// changed since the login
+			if ($loginTimestamp < $passwordTimestamp) {
+				$user->logout();
+				return null;
+			}
+		}
+
+		// in case the session needs to be updated, do it now
+		// for better performance
+		$session->commit();
+		return $user;
 	}
 
 	/**
 	 * Returns the list of enabled challenges in the
 	 * configured order
diff --git a/src/Cms/User.php b/src/Cms/User.php
index 8a8fc968e..d9d52b1e5 100644
--- a/src/Cms/User.php
+++ b/src/Cms/User.php
@@ -439,10 +439,13 @@ class User extends ModelWithContent
 
 		$kirby->trigger('user.login:before', ['user' => $this, 'session' => $session]);
 
 		$session->regenerateToken(); // privilege change
 		$session->data()->set('kirby.userId', $this->id());
+		if ($this->passwordTimestamp() !== null) {
+			$session->data()->set('kirby.loginTimestamp', time());
+		}
 		$this->kirby()->auth()->setUser($this);
 
 		$kirby->trigger('user.login:after', ['user' => $this, 'session' => $session]);
 	}
 
@@ -459,10 +462,11 @@ class User extends ModelWithContent
 
 		$kirby->trigger('user.logout:before', ['user' => $this, 'session' => $session]);
 
 		// remove the user from the session for future requests
 		$session->data()->remove('kirby.userId');
+		$session->data()->remove('kirby.loginTimestamp');
 
 		// clear the cached user object from the app state of the current request
 		$this->kirby()->auth()->flush();
 
 		if ($session->data()->get() === []) {
@@ -605,10 +609,30 @@ class User extends ModelWithContent
 		}
 
 		return $this->password = $this->readPassword();
 	}
 
+	/**
+	 * Returns the timestamp when the password
+	 * was last changed
+	 */
+	public function passwordTimestamp(): int|null
+	{
+		$file = $this->passwordFile();
+
+		// ensure we have the latest information
+		// to prevent cache attacks
+		clearstatcache();
+
+		// user does not have a password
+		if (is_file($file) === false) {
+			return null;
+		}
+
+		return filemtime($file);
+	}
+
 	/**
 	 * @return \Kirby\Cms\UserPermissions
 	 */
 	public function permissions()
 	{
@@ -879,6 +903,14 @@ class User extends ModelWithContent
 			throw new InvalidArgumentException(['key' => 'user.password.wrong', 'httpCode' => 401]);
 		}
 
 		return true;
 	}
+
+	/**
+	 * Returns the path to the password file
+	 */
+	protected function passwordFile(): string
+	{
+		return $this->root() . '/.htpasswd';
+	}
 }
diff --git a/src/Cms/UserActions.php b/src/Cms/UserActions.php
index 25e30fb85..2c89043c5 100644
--- a/src/Cms/UserActions.php
+++ b/src/Cms/UserActions.php
@@ -116,10 +116,17 @@ trait UserActions
 			$user->writePassword($password);
 
 			// update the users collection
 			$user->kirby()->users()->set($user->id(), $user);
 
+			// keep the user logged in to the current browser
+			// if they changed their own password
+			// (regenerate the session token, update the login timestamp)
+			if ($user->isLoggedIn() === true) {
+				$user->loginPasswordless();
+			}
+
 			return $user;
 		});
 	}
 
 	/**
@@ -321,11 +328,11 @@ trait UserActions
 	 *
 	 * @return string|false
 	 */
 	protected function readPassword()
 	{
-		return F::read($this->root() . '/.htpasswd');
+		return F::read($this->passwordFile());
 	}
 
 	/**
 	 * Updates the user data
 	 *
@@ -382,8 +389,8 @@ trait UserActions
 	 */
 	protected function writePassword(
 		#[SensitiveParameter]
 		string $password = null
 	): bool {
-		return F::write($this->root() . '/.htpasswd', $password);
+		return F::write($this->passwordFile(), $password);
 	}
 }
diff --git a/tests/Cms/Auth/AuthTest.php b/tests/Cms/Auth/AuthTest.php
index 6552c1cae..7a81a0969 100644
--- a/tests/Cms/Auth/AuthTest.php
+++ b/tests/Cms/Auth/AuthTest.php
@@ -3,27 +3,28 @@
 namespace Kirby\Cms;
 
 use Kirby\Exception\NotFoundException;
 use Kirby\Exception\PermissionException;
 use Kirby\Filesystem\Dir;
+use Kirby\Filesystem\F;
 use Kirby\Session\AutoSession;
 use Throwable;
 
 /**
  * @coversDefaultClass \Kirby\Cms\Auth
  */
 class AuthTest extends TestCase
 {
 	protected $app;
 	protected $auth;
-	protected $fixtures;
+	protected $tmp;
 
 	public function setUp(): void
 	{
 		$this->app = new App([
 			'roots' => [
-				'index' => $this->fixtures = __DIR__ . '/fixtures/AuthTest'
+				'index' => $this->tmp = __DIR__ . '/tmp'
 			],
 			'options' => [
 				'api' => [
 					'basicAuth'     => true,
 					'allowInsecure' => true
@@ -39,23 +40,25 @@ class AuthTest extends TestCase
 					'password' => password_hash('springfield123', PASSWORD_DEFAULT)
 				],
 				[
 					'email'    => 'homer@simpsons.com',
 					'id'       => 'homer',
-					'password' => password_hash('springfield123', PASSWORD_DEFAULT)
+					'password' => $hash = password_hash('springfield123', PASSWORD_DEFAULT)
 				]
 			]
 		]);
-		Dir::make($this->fixtures . '/site/accounts');
+		Dir::make($this->tmp . '/site/accounts/homer');
+		F::write($this->tmp . '/site/accounts/homer/.htpasswd', $hash);
+		touch($this->tmp . '/site/accounts/homer/.htpasswd', 1337000000);
 
 		$this->auth = $this->app->auth();
 	}
 
 	public function tearDown(): void
 	{
 		$this->app->session()->destroy();
-		Dir::remove($this->fixtures);
+		Dir::remove($this->tmp);
 		App::destroy();
 	}
 
 	/**
 	 * @covers ::currentUserFromImpersonation
@@ -252,11 +255,11 @@ class AuthTest extends TestCase
 
 	/**
 	 * @covers ::status
 	 * @covers ::user
 	 */
-	public function testUserSession1()
+	public function testUserSession()
 	{
 		$session = $this->app->session();
 		$session->set('kirby.userId', 'marge');
 
 		$user = $this->auth->user();
@@ -284,24 +287,66 @@ class AuthTest extends TestCase
 
 	/**
 	 * @covers ::status
 	 * @covers ::user
 	 */
-	public function testUserSession2()
+	public function testUserSessionManualSession()
 	{
 		$session = (new AutoSession($this->app->root('sessions')))->createManually();
 		$session->set('kirby.userId', 'homer');
+		$session->set('kirby.loginTimestamp', 1337000000);
 
 		$user = $this->auth->user($session);
 		$this->assertSame('homer@simpsons.com', $user->email());
 		$this->assertSame([
 			'challenge' => null,
 			'email'     => 'homer@simpsons.com',
 			'status'    => 'active'
 		], $this->auth->status()->toArray());
 	}
 
+	/**
+	 * @covers ::status
+	 * @covers ::user
+	 */
+	public function testUserSessionOldTimestamp()
+	{
+		$session = $this->app->session();
+		$session->set('kirby.userId', 'homer');
+		$session->set('kirby.loginTimestamp', 1000000000);
+
+		$this->assertNull($this->auth->user());
+		$this->assertSame([
+			'challenge' => null,
+			'email'     => null,
+			'status'    => 'inactive'
+		], $this->auth->status()->toArray());
+
+		// user should be logged out completely
+		$this->assertSame([], $session->data()->get());
+	}
+
+	/**
+	 * @covers ::status
+	 * @covers ::user
+	 */
+	public function testUserSessionNoTimestamp()
+	{
+		$session = $this->app->session();
+		$session->set('kirby.userId', 'homer');
+
+		$this->assertNull($this->auth->user());
+		$this->assertSame([
+			'challenge' => null,
+			'email'     => null,
+			'status'    => 'inactive'
+		], $this->auth->status()->toArray());
+
+		// user should be logged out completely
+		$this->assertSame([], $session->data()->get());
+	}
+
 	/**
 	 * @covers ::status
 	 * @covers ::user
 	 */
 	public function testUserBasicAuth()
diff --git a/tests/Cms/Users/UserActionsTest.php b/tests/Cms/Users/UserActionsTest.php
index 22ed58657..c9c803dd4 100644
--- a/tests/Cms/Users/UserActionsTest.php
+++ b/tests/Cms/Users/UserActionsTest.php
@@ -40,10 +40,11 @@ class UserActionsTest extends TestCase
 		]);
 	}
 
 	public function tearDown(): void
 	{
+		$this->app->session()->destroy();
 		Dir::remove($this->tmp);
 	}
 
 	public function testChangeEmail()
 	{
@@ -388,19 +389,57 @@ class UserActionsTest extends TestCase
 					$calls++;
 				},
 				'user.changePassword:after' => function (User $newUser, User $oldUser) use ($phpunit, &$calls) {
 					$phpunit->assertTrue($newUser->validatePassword('topsecret2018'));
 					$phpunit->assertEmpty($oldUser->password());
-					$calls++;
-				}
+					$calls += 2;
+				},
+				'user.login:before' => function () use (&$calls) {
+					$calls += 4;
+				},
+				'user.login:after' => function () use (&$calls) {
+					$calls += 8;
+				},
 			]
 		]);
 
 		$user = $app->user('editor@domain.com');
 		$user->changePassword('topsecret2018');
 
-		$this->assertSame(2, $calls);
+		$this->assertSame(3, $calls);
+	}
+
+	public function testChangePasswordHooksCurrentUser()
+	{
+		$calls = 0;
+		$phpunit = $this;
+
+		$this->app = $this->app->clone([
+			'hooks' => [
+				'user.changePassword:before' => function (User $user, $password) use ($phpunit, &$calls) {
+					$phpunit->assertEmpty($user->password());
+					$phpunit->assertSame('topsecret2018', $password);
+					$calls++;
+				},
+				'user.changePassword:after' => function (User $newUser, User $oldUser) use ($phpunit, &$calls) {
+					$phpunit->assertTrue($newUser->validatePassword('topsecret2018'));
+					$phpunit->assertEmpty($oldUser->password());
+					$calls += 2;
+				},
+				'user.login:before' => function () use (&$calls) {
+					$calls += 4;
+				},
+				'user.login:after' => function () use (&$calls) {
+					$calls += 8;
+				},
+			]
+		]);
+
+		$user = $this->app->user('admin@domain.com');
+		$user->changePassword('topsecret2018');
+
+		$this->assertSame(15, $calls);
 	}
 
 	public function testChangeRoleHooks()
 	{
 		$calls = 0;
diff --git a/tests/Cms/Users/UserAuthTest.php b/tests/Cms/Users/UserAuthTest.php
index 113afb916..6bfde4e5f 100644
--- a/tests/Cms/Users/UserAuthTest.php
+++ b/tests/Cms/Users/UserAuthTest.php
@@ -1,9 +1,11 @@
 <?php
 
 namespace Kirby\Cms;
 
+use Kirby\Filesystem\F;
+
 class UserAuthTest extends TestCase
 {
 	protected $app;
 	protected $tmp = __DIR__ . '/tmp';
 
@@ -17,10 +19,11 @@ class UserAuthTest extends TestCase
 				'sessions' => $this->tmp . '/sessions'
 			],
 			'users' => [
 				[
 					'email' => 'test@getkirby.com',
+					'id'    => 'testuser',
 					'role'  => 'admin'
 				]
 			]
 		]);
 	}
@@ -94,6 +97,32 @@ class UserAuthTest extends TestCase
 		$user->logout();
 
 		// each hook needs to be called exactly twice
 		$this->assertSame((1 + 2 + 4 + 8) * 2, $calls);
 	}
+
+	public function testSessionData()
+	{
+		$user    = $this->app->user('test@getkirby.com');
+		$session = $this->app->session();
+
+		$this->assertSame([], $session->data()->get());
+		$user->loginPasswordless();
+		$this->assertSame(['kirby.userId' => 'testuser'], $session->data()->get());
+		$user->logout();
+		$this->assertSame([], $session->data()->get());
+	}
+
+	public function testSessionDataWithPassword()
+	{
+		F::write($this->tmp . '/accounts/testuser/.htpasswd', 'a very secure hash');
+
+		$user    = $this->app->user('test@getkirby.com');
+		$session = $this->app->session();
+
+		$this->assertSame([], $session->data()->get());
+		$user->loginPasswordless();
+		$this->assertSame(['kirby.userId' => 'testuser', 'kirby.loginTimestamp' => 1337000000], $session->data()->get());
+		$user->logout();
+		$this->assertSame([], $session->data()->get());
+	}
 }
diff --git a/tests/Cms/Users/UserTest.php b/tests/Cms/Users/UserTest.php
index b193d1541..bf85e421d 100644
--- a/tests/Cms/Users/UserTest.php
+++ b/tests/Cms/Users/UserTest.php
@@ -191,10 +191,36 @@ class UserTest extends TestCase
 		$this->assertSame((string)$modifiedDeContent, $user->modified('U', null, 'de'));
 
 		Dir::remove($index);
 	}
 
+	public function testPasswordTimestamp()
+	{
+		$app = new App([
+			'roots' => [
+				'index'    => $this->tmp,
+				'accounts' => $this->tmp
+			]
+		]);
+
+		// create a user file
+		F::write($this->tmp . '/test/index.php', '<?php return [];');
+
+		$user = $app->user('test');
+		$this->assertNull($user->passwordTimestamp());
+
+		// create a password file
+		F::write($this->tmp . '/test/.htpasswd', 'a very secure hash');
+		touch($this->tmp . '/test/.htpasswd', 1337000000);
+
+		$this->assertSame(1337000000, $user->passwordTimestamp());
+
+		// timestamp is not cached
+		touch($this->tmp . '/test/.htpasswd', 1338000000);
+		$this->assertSame(1338000000, $user->passwordTimestamp());
+	}
+
 	public function passwordProvider()
 	{
 		return [
 			[null, false],
 			['', false],

Vulnerable code: 		if ($user = $this->kirby->users()->find($id)) {
			// in case the session needs to be updated, do it now
			// for better performance
			$session->commit();
			return $user;
		return null;
		return F::read($this->root() . '/.htpasswd');
		return F::write($this->root() . '/.htpasswd', $password);
	protected $fixtures;
				'index' => $this->fixtures = __DIR__ . '/fixtures/AuthTest'
					'password' => password_hash('springfield123', PASSWORD_DEFAULT)
		Dir::make($this->fixtures . '/site/accounts');
		Dir::remove($this->fixtures);
	public function testUserSession1()
	public function testUserSession2()
					$calls++;
				}
		$this->assertSame(2, $calls);
Fixed code: 		// if no user is logged in, return immediately
		// a user is logged in, ensure it exists
		$user = $this->kirby->users()->find($id);
		if ($user === null) {
			return null;
		if ($passwordTimestamp = $user->passwordTimestamp()) {
			$loginTimestamp = $session->data()->get('kirby.loginTimestamp');
			if (is_int($loginTimestamp) !== true) {
				// session that was created before Kirby
				// 3.5.8.3, 3.6.6.3, 3.7.5.2, 3.8.4.1 or 3.9.6
				// or when the user didn't have a password set
				$user->logout();
				return null;
			}

			// invalidate the session if the password
			// changed since the login
			if ($loginTimestamp < $passwordTimestamp) {
				$user->logout();
				return null;
			}
		}

		// in case the session needs to be updated, do it now
		// for better performance
		$session->commit();
		return $user;
		if ($this->passwordTimestamp() !== null) {
			$session->data()->set('kirby.loginTimestamp', time());
		}
		$session->data()->remove('kirby.loginTimestamp');
	/**
	 * Returns the timestamp when the password
	 * was last changed
	 */
	public function passwordTimestamp(): int|null
	{
		$file = $this->passwordFile();

		// ensure we have the latest information
		// to prevent cache attacks
		clearstatcache();

		// user does not have a password
		if (is_file($file) === false) {
			return null;
		}

		return filemtime($file);
	}


	/**
	 * Returns the path to the password file
	 */
	protected function passwordFile(): string
	{
		return $this->root() . '/.htpasswd';
	}
			// keep the user logged in to the current browser
			// if they changed their own password
			// (regenerate the session token, update the login timestamp)
			if ($user->isLoggedIn() === true) {
				$user->loginPasswordless();
			}

		return F::read($this->passwordFile());
		return F::write($this->passwordFile(), $password);
use Kirby\Filesystem\F;
	protected $tmp;
				'index' => $this->tmp = __DIR__ . '/tmp'
					'password' => $hash = password_hash('springfield123', PASSWORD_DEFAULT)
		Dir::make($this->tmp . '/site/accounts/homer');
		F::write($this->tmp . '/site/accounts/homer/.htpasswd', $hash);
		touch($this->tmp . '/site/accounts/homer/.htpasswd', 1337000000);
		Dir::remove($this->tmp);
	public function testUserSession()
	public function testUserSessionManualSession()
		$session->set('kirby.loginTimestamp', 1337000000);
	/**
	 * @covers ::status
	 * @covers ::user
	 */
	public function testUserSessionOldTimestamp()
	{
		$session = $this->app->session();
		$session->set('kirby.userId', 'homer');
		$session->set('kirby.loginTimestamp', 1000000000);

		$this->assertNull($this->auth->user());
		$this->assertSame([
			'challenge' => null,
			'email'     => null,
			'status'    => 'inactive'
		], $this->auth->status()->toArray());

		// user should be logged out completely
		$this->assertSame([], $session->data()->get());
	}

	/**
	 * @covers ::status
	 * @covers ::user
	 */
	public function testUserSessionNoTimestamp()
	{
		$session = $this->app->session();
		$session->set('kirby.userId', 'homer');

		$this->assertNull($this->auth->user());
		$this->assertSame([
			'challenge' => null,
			'email'     => null,
			'status'    => 'inactive'
		], $this->auth->status()->toArray());

		// user should be logged out completely
		$this->assertSame([], $session->data()->get());
	}

		$this->app->session()->destroy();
					$calls += 2;
				},
				'user.login:before' => function () use (&$calls) {
					$calls += 4;
				},
				'user.login:after' => function () use (&$calls) {
					$calls += 8;
				},
		$this->assertSame(3, $calls);
	}

	public function testChangePasswordHooksCurrentUser()
	{
		$calls = 0;
		$phpunit = $this;

		$this->app = $this->app->clone([
			'hooks' => [
				'user.changePassword:before' => function (User $user, $password) use ($phpunit, &$calls) {
					$phpunit->assertEmpty($user->password());
					$phpunit->assertSame('topsecret2018', $password);
					$calls++;
				},
				'user.changePassword:after' => function (User $newUser, User $oldUser) use ($phpunit, &$calls) {
					$phpunit->assertTrue($newUser->validatePassword('topsecret2018'));
					$phpunit->assertEmpty($oldUser->password());
					$calls += 2;
				},
				'user.login:before' => function () use (&$calls) {
					$calls += 4;
				},
				'user.login:after' => function () use (&$calls) {
					$calls += 8;
				},
			]
		]);

		$user = $this->app->user('admin@domain.com');
		$user->changePassword('topsecret2018');

		$this->assertSame(15, $calls);
use Kirby\Filesystem\F;

					'id'    => 'testuser',

	public function testSessionData()
	{
		$user    = $this->app->user('test@getkirby.com');
		$session = $this->app->session();

		$this->assertSame([], $session->data()->get());
		$user->loginPasswordless();
		$this->assertSame(['kirby.userId' => 'testuser'], $session->data()->get());
		$user->logout();
		$this->assertSame([], $session->data()->get());
	}

	public function testSessionDataWithPassword()
	{
		F::write($this->tmp . '/accounts/testuser/.htpasswd', 'a very secure hash');

		$user    = $this->app->user('test@getkirby.com');
		$session = $this->app->session();

		$this->assertSame([], $session->data()->get());
		$user->loginPasswordless();
		$this->assertSame(['kirby.userId' => 'testuser', 'kirby.loginTimestamp' => 1337000000], $session->data()->get());
		$user->logout();
		$this->assertSame([], $session->data()->get());
	}
	public function testPasswordTimestamp()
	{
		$app = new App([
			'roots' => [
				'index'    => $this->tmp,
				'accounts' => $this->tmp
			]
		]);

		// create a user file
		F::write($this->tmp . '/test/index.php', '<?php return [];');

		$user = $app->user('test');
		$this->assertNull($user->passwordTimestamp());

		// create a password file
		F::write($this->tmp . '/test/.htpasswd', 'a very secure hash');
		touch($this->tmp . '/test/.htpasswd', 1337000000);

		$this->assertSame(1337000000, $user->passwordTimestamp());

		// timestamp is not cached
		touch($this->tmp . '/test/.htpasswd', 1338000000);
		$this->assertSame(1338000000, $user->passwordTimestamp());
	}