From 6b8a1e13c6b02359c79ff3c07e0f5b8921c72498 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 16 Mar 2021 10:16:03 +0100
Subject: [PATCH] [SECURITY] Avoid storing plain session identifier in
$USER->uc
`AbstractUserAuthentication::$uc['moduleSessionID']` still stored plain
session identifier, which has been replaced by corresponding HMAC.
Resolves: #93359
Releases: master, 11.1, 10.4, 9.5
Change-Id: I920b8d3b364c249d2ec3a6deb42e141e5a1b8ff7
Security-Bulletin: TYPO3-CORE-SA-2021-006
Security-References: CVE-2021-21339
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/68439
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
.../AbstractUserAuthentication.php | 20 ++++-
.../AbstractUserAuthenticationTest.php | 84 +++++++++++++++++++
.../Fixtures/AnyUserAuthentication.php | 40 +++++++++
3 files changed, 141 insertions(+), 3 deletions(-)
create mode 100644 typo3/sysext/core/Tests/Functional/Authentication/AbstractUserAuthenticationTest.php
create mode 100644 typo3/sysext/core/Tests/Functional/Authentication/Fixtures/AnyUserAuthentication.php
diff --git a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
index 5cb97907446f..f531817f82cb 100644
--- a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
@@ -1019,8 +1019,12 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
*/
public function pushModuleData($module, $data, $noSave = 0)
{
+ $sessionHash = GeneralUtility::hmac(
+ $this->userSession->getIdentifier(),
+ 'core-session-hash'
+ );
$this->uc['moduleData'][$module] = $data;
- $this->uc['moduleSessionID'][$module] = $this->userSession->getIdentifier();
+ $this->uc['moduleSessionID'][$module] = $sessionHash;
if (!$noSave) {
$this->writeUC();
}
@@ -1035,8 +1039,18 @@ abstract class AbstractUserAuthentication implements LoggerAwareInterface
*/
public function getModuleData($module, $type = '')
{
- if ($type !== 'ses' || (isset($this->uc['moduleSessionID'][$module]) && $this->uc['moduleSessionID'][$module] == $this->userSession->getIdentifier())) {
- return $this->uc['moduleData'][$module];
+ $sessionHash = GeneralUtility::hmac(
+ $this->userSession->getIdentifier(),
+ 'core-session-hash'
+ );
+ $sessionData = $this->uc['moduleData'][$module] ?? null;
+ $moduleSessionIdHash = $this->uc['moduleSessionID'][$module] ?? null;
+ if ($type !== 'ses'
+ || $sessionData !== null && $moduleSessionIdHash === $sessionHash
+ // @todo Fallback for non-hashed values in `moduleSessionID`, remove for TYPO3 v11.5 LTS
+ || $sessionData !== null && $moduleSessionIdHash === $this->userSession->getIdentifier()
+ ) {
+ return $sessionData;
}
return null;
}
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/AbstractUserAuthenticationTest.php b/typo3/sysext/core/Tests/Functional/Authentication/AbstractUserAuthenticationTest.php
new file mode 100644
index 000000000000..a3b193581acb
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Authentication/AbstractUserAuthenticationTest.php
@@ -0,0 +1,84 @@
+<?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\Tests\Functional\Authentication;
+
+use TYPO3\CMS\Core\Session\UserSession;
+use TYPO3\CMS\Core\Tests\Functional\Authentication\Fixtures\AnyUserAuthentication;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+class AbstractUserAuthenticationTest extends FunctionalTestCase
+{
+ /**
+ * @var string
+ */
+ private $sessionId;
+
+ /**
+ * @var AnyUserAuthentication
+ */
+ private $subject;
+
+ /**
+ * @var UserSession
+ */
+ private $userSession;
+
+ protected function setUp(): void
+ {
+ parent::setUp();
+ $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = '12345';
+ $this->sessionId = bin2hex(random_bytes(20));
+ $this->userSession = UserSession::createNonFixated($this->sessionId);
+ $this->subject = new AnyUserAuthentication($this->userSession);
+ }
+
+ protected function tearDown(): void
+ {
+ unset($this->sessionId, $this->userSession, $this->subject);
+ unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']);
+ parent::tearDown();
+ }
+
+ /**
+ * @test
+ */
+ public function pushModuleDataDoesNotRevealPlainSessionId(): void
+ {
+ $this->subject->pushModuleData(self::class, true);
+ self::assertNotContains($this->sessionId, $this->subject->uc['moduleSessionID']);
+ }
+
+ /**
+ * @test
+ */
+ public function getModuleDataResolvesHashedSessionId(): void
+ {
+ $this->subject->pushModuleData(self::class, true);
+ self::assertTrue($this->subject->getModuleData(self::class));
+ }
+
+ /**
+ * @test
+ */
+ public function getModuleDataFallsBackToPlainSessionId(): void
+ {
+ $this->subject->uc['moduleData'][self::class] = true;
+ $this->subject->uc['moduleSessionID'][self::class] = $this->sessionId;
+ self::assertTrue($this->subject->getModuleData(self::class));
+ }
+}
diff --git a/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/AnyUserAuthentication.php b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/AnyUserAuthentication.php
new file mode 100644
index 000000000000..321cb614d013
--- /dev/null
+++ b/typo3/sysext/core/Tests/Functional/Authentication/Fixtures/AnyUserAuthentication.php
@@ -0,0 +1,40 @@
+<?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\Tests\Functional\Authentication\Fixtures;
+
+use TYPO3\CMS\Core\Authentication\AbstractUserAuthentication;
+use TYPO3\CMS\Core\Session\UserSession;
+
+class AnyUserAuthentication extends AbstractUserAuthentication
+{
+ /**
+ * @var array
+ */
+ public $uc = [];
+
+ /**
+ * @var string
+ */
+ public $loginType = 'ANY';
+
+ public function __construct(UserSession $userSession)
+ {
+ parent::__construct();
+ $this->userSession = $userSession;
+ }
+}
--
GitLab