From 8fdec565611f1728b69178bd0fbd2e978858fff2 Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <ben@bnf.dev>
Date: Tue, 14 Nov 2023 09:57:14 +0100
Subject: [PATCH] [SECURITY] Limit user session to cookie domain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Given that there are two sites `site-a.com` and `site-b.com` in
the same TYPO3 installation, it was possible to reuse a session
cookie that was generated for `site-a.com` in `site-b.com`.
Since there are scenarios, where this is the expected behavior
– when sharing sessions across sub domains, so that an explicit
cookieDomain needs to be configured – user sessions IDs are now
signed with a combination of encryption key and desired cookie
domain, so that a cookie can only be used on the domain that the
cookie was created for.
For compatiblity with possible 3rd party authenticators, legacy
tokens will be accepted (but not created by TYPO3 core itself).
Resolves: #100885
Releases: main, 12.4, 11.5
Change-Id: I0d1c314c6e206ac12604ba6f859af78b958651dd
Security-Bulletin: TYPO3-CORE-SA-2023-006
Security-References: CVE-2023-47127
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81731
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
.../core/Classes/Http/CookieScopeTrait.php | 67 +++++++++++++++++++
.../Classes/Session/UserSessionManager.php | 59 ++++++++++++++--
.../Functional/Page/PageRendererTest.php | 3 +-
.../BackendUserAuthenticationTest.php | 3 +-
.../Unit/Session/UserSessionManagerTest.php | 32 +++++++--
.../FrontendUserAuthenticationTest.php | 3 +-
6 files changed, 153 insertions(+), 14 deletions(-)
create mode 100644 typo3/sysext/core/Classes/Http/CookieScopeTrait.php
diff --git a/typo3/sysext/core/Classes/Http/CookieScopeTrait.php b/typo3/sysext/core/Classes/Http/CookieScopeTrait.php
new file mode 100644
index 000000000000..e5ddad67bc1e
--- /dev/null
+++ b/typo3/sysext/core/Classes/Http/CookieScopeTrait.php
@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Http;
+
+trait CookieScopeTrait
+{
+ /**
+ * Returns the domain and path to be used for setting cookies.
+ * The information is taken from the value in $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'] if set,
+ * otherwise the normalized request params are used.
+ *
+ * @return array{domain: string, path: string} The domain and path to be used when setting cookies
+ */
+ private function getCookieScope(NormalizedParams $normalizedParams): array
+ {
+ $cookieDomain = $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'] ?? '';
+ // If a specific cookie domain is defined for a given application type, use that domain
+ if (!empty($GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'])) {
+ $cookieDomain = $GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'];
+ }
+ if (!$cookieDomain) {
+ return [
+ 'domain' => $normalizedParams->getRequestHostOnly(),
+ // If no cookie domain is set, use the base path
+ 'path' => $normalizedParams->getSitePath(),
+ ];
+ }
+ if ($cookieDomain[0] === '/') {
+ $match = [];
+ $matchCount = @preg_match($cookieDomain, $normalizedParams->getRequestHostOnly(), $match);
+ if ($matchCount === false) {
+ $this->logger->critical(
+ 'The regular expression for the cookie domain ({domain}) contains errors. The session is not shared across sub-domains.',
+ ['domain' => $cookieDomain]
+ );
+ }
+ if ($matchCount === false || $matchCount === 0) {
+ return [
+ 'domain' => $normalizedParams->getRequestHostOnly(),
+ // If no cookie domain could be matched, use the base path
+ 'path' => $normalizedParams->getSitePath(),
+ ];
+ }
+ $cookieDomain = $match[0];
+ }
+
+ return [
+ 'domain' => trim($cookieDomain, '.'),
+ 'path' => '/',
+ ];
+ }
+}
diff --git a/typo3/sysext/core/Classes/Session/UserSessionManager.php b/typo3/sysext/core/Classes/Session/UserSessionManager.php
index d4a60410f400..cb69de751cf6 100644
--- a/typo3/sysext/core/Classes/Session/UserSessionManager.php
+++ b/typo3/sysext/core/Classes/Session/UserSessionManager.php
@@ -21,7 +21,10 @@ use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
use TYPO3\CMS\Core\Authentication\IpLocker;
+use TYPO3\CMS\Core\Core\Environment;
use TYPO3\CMS\Core\Crypto\Random;
+use TYPO3\CMS\Core\Http\CookieScopeTrait;
+use TYPO3\CMS\Core\Http\NormalizedParams;
use TYPO3\CMS\Core\Session\Backend\Exception\SessionNotFoundException;
use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -34,6 +37,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
class UserSessionManager implements LoggerAwareInterface
{
use LoggerAwareTrait;
+ use CookieScopeTrait;
protected const SESSION_ID_LENGTH = 32;
protected const GARBAGE_COLLECTION_LIFETIME = 86400;
@@ -52,6 +56,7 @@ class UserSessionManager implements LoggerAwareInterface
protected int $garbageCollectionForAnonymousSessions = self::LIFETIME_OF_ANONYMOUS_SESSION_DATA;
protected SessionBackendInterface $sessionBackend;
protected IpLocker $ipLocker;
+ protected string $loginType;
/**
* Constructor. Marked as internal, as it is recommended to use the factory method "create"
@@ -61,11 +66,12 @@ class UserSessionManager implements LoggerAwareInterface
* @param IpLocker $ipLocker
* @internal
*/
- public function __construct(SessionBackendInterface $sessionBackend, int $sessionLifetime, IpLocker $ipLocker)
+ public function __construct(SessionBackendInterface $sessionBackend, int $sessionLifetime, IpLocker $ipLocker, string $loginType)
{
$this->sessionBackend = $sessionBackend;
$this->sessionLifetime = $sessionLifetime;
$this->ipLocker = $ipLocker;
+ $this->loginType = $loginType;
}
protected function setGarbageCollectionTimeoutForAnonymousSessions(int $garbageCollectionForAnonymousSessions = 0): void
@@ -285,13 +291,38 @@ class UserSessionManager implements LoggerAwareInterface
}
/**
- * Creates a new session ID using a random with SESSION_ID_LENGTH as length
+ * Creates a new session ID using a random with SESSION_ID_LENGTH as length of the random part
*
* @return string
*/
protected function createSessionId(): string
{
- return GeneralUtility::makeInstance(Random::class)->generateRandomHexString(self::SESSION_ID_LENGTH);
+ $normalizedParams = $this->getNormalizedParams();
+ $cookieScope = $this->getCookieScope($normalizedParams);
+ $key = sha1($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] . '/' . UserSession::class . '/' . $cookieScope['domain']);
+ $random = GeneralUtility::makeInstance(Random::class)->generateRandomHexString(self::SESSION_ID_LENGTH);
+ $signature = hash_hmac('sha256', $random, $key);
+
+ return $random . '.' . $signature;
+ }
+
+ /**
+ * @todo/notes for backports: Same as in typo3/sysext/core/Classes/Hooks/CreateSiteConfiguration.php,
+ */
+ protected function getNormalizedParams(): NormalizedParams
+ {
+ $normalizedParams = null;
+ $serverParams = Environment::isCli() ? ['HTTP_HOST' => 'localhost'] : $_SERVER;
+ if (isset($GLOBALS['TYPO3_REQUEST'])) {
+ $normalizedParams = $GLOBALS['TYPO3_REQUEST']->getAttribute('normalizedParams');
+ $serverParams = $GLOBALS['TYPO3_REQUEST']->getServerParams();
+ }
+
+ if (!$normalizedParams instanceof NormalizedParams) {
+ $normalizedParams = NormalizedParams::createFromServerParams($serverParams);
+ }
+
+ return $normalizedParams;
}
/**
@@ -306,6 +337,25 @@ class UserSessionManager implements LoggerAwareInterface
if ($id === '') {
return null;
}
+
+ $sessionsParts = explode('.', $id, 2);
+ // Verify if session id is signed with cookie domain.
+ // Note that we allow possibly unsiged session IDs (used for testing framework or 3rd party authenticators)
+ if (count($sessionsParts) === 2) {
+ $random = $sessionsParts[0];
+ $signature = $sessionsParts[1];
+ $normalizedParams = $this->getNormalizedParams();
+ $cookieScope = $this->getCookieScope($normalizedParams);
+ $key = sha1($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] . '/' . UserSession::class . '/' . $cookieScope['domain']);
+ $validHash = hash_hmac('sha256', $random, $key);
+ if (!hash_equals($validHash, $signature)) {
+ $this->logger->notice('User Session rejected because of invalid signature', ['session' => substr(sha1($id), 0, 12)]);
+ return null;
+ }
+ } elseif ($this->logger !== null) {
+ $this->logger->notice('Unsigned session id has been used', ['session' => substr(sha1($id), 0, 12)]);
+ }
+
try {
$sessionRecord = $this->sessionBackend->get($id);
if ($sessionRecord === []) {
@@ -357,7 +407,8 @@ class UserSessionManager implements LoggerAwareInterface
self::class,
$sessionManager->getSessionBackend($loginType),
$sessionLifetime,
- $ipLocker
+ $ipLocker,
+ $loginType
);
if ($loginType === 'FE') {
$object->setGarbageCollectionTimeoutForAnonymousSessions((int)($GLOBALS['TYPO3_CONF_VARS']['FE']['sessionDataLifetime'] ?? 0));
diff --git a/typo3/sysext/core/Tests/Functional/Page/PageRendererTest.php b/typo3/sysext/core/Tests/Functional/Page/PageRendererTest.php
index 293793a625af..6bcd200bd457 100644
--- a/typo3/sysext/core/Tests/Functional/Page/PageRendererTest.php
+++ b/typo3/sysext/core/Tests/Functional/Page/PageRendererTest.php
@@ -341,7 +341,8 @@ class PageRendererTest extends FunctionalTestCase
$userSessionManager = new UserSessionManager(
$sessionBackend->reveal(),
86400,
- $this->prophesize(IpLocker::class)->reveal()
+ $this->prophesize(IpLocker::class)->reveal(),
+ 'BE'
);
$GLOBALS['BE_USER'] = new BackendUserAuthentication();
$GLOBALS['BE_USER']->initializeUserSessionManager($userSessionManager);
diff --git a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
index 488eb9b2d3c8..83a039e683e3 100644
--- a/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
+++ b/typo3/sysext/core/Tests/Unit/Authentication/BackendUserAuthenticationTest.php
@@ -101,7 +101,8 @@ class BackendUserAuthenticationTest extends UnitTestCase
$userSessionManager = new UserSessionManager(
$sessionBackend->reveal(),
86400,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'BE'
);
$GLOBALS['BE_USER'] = $this->getMockBuilder(BackendUserAuthentication::class)->getMock();
diff --git a/typo3/sysext/core/Tests/Unit/Session/UserSessionManagerTest.php b/typo3/sysext/core/Tests/Unit/Session/UserSessionManagerTest.php
index 9a66feb644ae..eaa8c6ceac61 100644
--- a/typo3/sysext/core/Tests/Unit/Session/UserSessionManagerTest.php
+++ b/typo3/sysext/core/Tests/Unit/Session/UserSessionManagerTest.php
@@ -21,6 +21,7 @@ use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\Authentication\IpLocker;
+use TYPO3\CMS\Core\Http\NormalizedParams;
use TYPO3\CMS\Core\Session\Backend\Exception\SessionNotFoundException;
use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
use TYPO3\CMS\Core\Session\UserSession;
@@ -62,7 +63,8 @@ class UserSessionManagerTest extends UnitTestCase
$subject = new UserSessionManager(
$sessionBackendProphecy->reveal(),
$sessionLifetime,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$session = $subject->createAnonymousSession();
self::assertEquals($expectedResult, $subject->willExpire($session, $gracePeriod));
@@ -75,7 +77,8 @@ class UserSessionManagerTest extends UnitTestCase
$subject = new UserSessionManager(
$sessionBackendProphecy->reveal(),
60,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$expiredSession = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
self::assertTrue($subject->hasExpired($expiredSession));
@@ -88,9 +91,16 @@ class UserSessionManagerTest extends UnitTestCase
*/
public function createFromRequestOrAnonymousCreatesProperSessionObjects(): void
{
+ $cookieDomain = 'example.org';
+ $normalizedParams = $this->createMock(NormalizedParams::class);
+ $normalizedParams->method('getRequestHostOnly')->willReturn($cookieDomain);
+ $key = sha1($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] . '/' . UserSession::class . '/' . $cookieDomain);
+ $sessionId = 'valid-session';
+ $signature = hash_hmac('sha256', $sessionId, $key);
+ $validSession = $sessionId . '.' . $signature;
$sessionBackendProphecy = $this->prophesize(SessionBackendInterface::class);
$sessionBackendProphecy->get('invalid-session')->willThrow(SessionNotFoundException::class);
- $sessionBackendProphecy->get('valid-session')->willReturn([
+ $sessionBackendProphecy->get($validSession)->willReturn([
'ses_id' => 'valid-session',
'ses_userid' => 13,
'ses_data' => serialize(['propertyA' => 42, 'propertyB' => 'great']),
@@ -100,18 +110,24 @@ class UserSessionManagerTest extends UnitTestCase
$subject = new UserSessionManager(
$sessionBackendProphecy->reveal(),
50,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$request = $this->prophesize(ServerRequestInterface::class);
$request->getCookieParams()->willReturn([]);
+ $request->getServerParams()->willReturn(['HTTP_HOST' => $cookieDomain]);
+ $request->getAttribute('normalizedParams')->willReturn($normalizedParams);
+ $GLOBALS['TYPO3_REQUEST'] = $request->reveal();
$anonymousSession = $subject->createFromRequestOrAnonymous($request->reveal(), 'foo');
self::assertTrue($anonymousSession->isNew());
self::assertTrue($anonymousSession->isAnonymous());
- $request->getCookieParams()->willReturn(['foo' => 'invalid-session', 'bar' => 'valid-session']);
+
+ $request->getCookieParams()->willReturn(['foo' => 'invalid-session', 'bar' => $validSession]);
$anonymousSessionFromInvalidBackendRequest = $subject->createFromRequestOrAnonymous($request->reveal(), 'foo');
self::assertTrue($anonymousSessionFromInvalidBackendRequest->isNew());
self::assertTrue($anonymousSessionFromInvalidBackendRequest->isAnonymous());
$persistedSession = $subject->createFromRequestOrAnonymous($request->reveal(), 'bar');
+
self::assertEquals(13, $persistedSession->getUserId());
self::assertFalse($persistedSession->isAnonymous());
self::assertFalse($persistedSession->isNew());
@@ -136,7 +152,8 @@ class UserSessionManagerTest extends UnitTestCase
$subject = new UserSessionManager(
$sessionBackendProphecy->reveal(),
60,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$session = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
$session = $subject->updateSession($session);
@@ -159,7 +176,8 @@ class UserSessionManagerTest extends UnitTestCase
$subject = new UserSessionManager(
$sessionBackendProphecy->reveal(),
60,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$session = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
$session = $subject->fixateAnonymousSession($session);
diff --git a/typo3/sysext/frontend/Tests/Unit/Authentication/FrontendUserAuthenticationTest.php b/typo3/sysext/frontend/Tests/Unit/Authentication/FrontendUserAuthenticationTest.php
index 5ab87b28e09a..3e232765bb28 100644
--- a/typo3/sysext/frontend/Tests/Unit/Authentication/FrontendUserAuthenticationTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/Authentication/FrontendUserAuthenticationTest.php
@@ -92,7 +92,8 @@ class FrontendUserAuthenticationTest extends UnitTestCase
$userSessionManager = new UserSessionManager(
$sessionBackendProphecy->reveal(),
86400,
- new IpLocker(0, 0)
+ new IpLocker(0, 0),
+ 'FE'
);
$subject = new FrontendUserAuthentication();
$subject->setLogger(new NullLogger());
--
GitLab