From fa12667c046342ebfd9b159c646aeafdbc52fcfd Mon Sep 17 00:00:00 2001 From: Benjamin Franzke Date: Tue, 13 Feb 2024 10:04:52 +0100 Subject: [PATCH] [SECURITY] Do not disclose encryptionKey via InstallTool The encryptionKey is a secret that must never be sent within any request, therefore it is now dropped from the editing interface in "Configure Installation-Wide Options". Resolves: #103046 Releases: main, 13.0, 12.4, 11.5 Change-Id: I260a8a2e9af29908543dfe48ac3658d8c45cc440 Security-Bulletin: TYPO3-CORE-SA-2024-004 Security-References: CVE-2024-25119 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82942 Reviewed-by: Oliver Hader Tested-by: Oliver Hader --- .../Classes/Configuration/ConfigurationManager.php | 1 + typo3/sysext/core/Classes/Log/Writer/FileWriter.php | 12 +++++++++++- .../core/Configuration/DefaultConfiguration.php | 1 - .../DefaultConfigurationDescription.yaml | 3 --- .../UnitDeprecated/Cache/Backend/PdoBackendTest.php | 12 ++++++++++++ .../ContentObject/ContentObjectRendererTest.php | 8 ++++++++ 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php b/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php index 5687d0e98d6b..6d0bf7e1d563 100644 --- a/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php +++ b/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php @@ -78,6 +78,7 @@ class ConfigurationManager 'EXTCONF', 'DB', 'SYS/caching/cacheConfigurations', + 'SYS/encryptionKey', 'SYS/session', 'EXTENSIONS', ]; diff --git a/typo3/sysext/core/Classes/Log/Writer/FileWriter.php b/typo3/sysext/core/Classes/Log/Writer/FileWriter.php index 1bd4f288e4b0..5e35346eeee2 100644 --- a/typo3/sysext/core/Classes/Log/Writer/FileWriter.php +++ b/typo3/sysext/core/Classes/Log/Writer/FileWriter.php @@ -68,7 +68,10 @@ public function __construct(array $options = []) { // the parent constructor reads $options and sets them parent::__construct($options); - if (empty($options['logFile'])) { + if (empty($options['logFile']) && + // omit logging if TYPO3 has not been configured (avoid creating a guessable filename) + ($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] ?? '') !== '' + ) { $this->setLogFile($this->getDefaultLogFileName()); } } @@ -78,6 +81,9 @@ public function __construct(array $options = []) */ public function __destruct() { + if ($this->logFile === '') { + return; + } self::$logFileHandlesCount[$this->logFile]--; if (self::$logFileHandlesCount[$this->logFile] <= 0) { $this->closeLogFile(); @@ -132,6 +138,10 @@ public function getLogFile(): string */ public function writeLog(LogRecord $record) { + if ($this->logFile === '') { + return $this; + } + $data = ''; $context = $record->getData(); $message = $record->getMessage(); diff --git a/typo3/sysext/core/Configuration/DefaultConfiguration.php b/typo3/sysext/core/Configuration/DefaultConfiguration.php index d3f6b723d174..6e2bee55b882 100644 --- a/typo3/sysext/core/Configuration/DefaultConfiguration.php +++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php @@ -83,7 +83,6 @@ ], 'createGroup' => '', 'sitename' => 'TYPO3', - 'encryptionKey' => '', 'cookieDomain' => '', 'trustedHostsPattern' => 'SERVER_NAME', 'devIPmask' => '127.0.0.1,::1', diff --git a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml index 3220aa667791..08700cef2bdc 100644 --- a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml +++ b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml @@ -80,9 +80,6 @@ SYS: sitename: type: text description: 'Name of the base-site.' - encryptionKey: - type: text - description: 'This is a "salt" used for various kinds of encryption, CRC checksums and validations. You can enter any rubbish string here but try to keep it secret. You should notice that a change to this value might invalidate temporary information, URLs etc. At least, clear all cache if you change this so any such information can be rebuilt with the new key.' cookieDomain: type: text description: 'Restricts the domain name for FE and BE session cookies. When setting the value to ".domain.com" (replace domain.com with your domain!), login sessions will be shared across subdomains. Alternatively, if you have more than one domain with sub-domains, you can set the value to a regular expression to match against the domain of the HTTP request. The result of the match is used as the domain for the cookie. eg. /\.(example1|example2)\.com$/ or /\.(example1\.com)|(example2\.net)$/. Separate domains for FE and BE can be set using $TYPO3_CONF_VARS[''FE''][''cookieDomain''] and $TYPO3_CONF_VARS[''BE''][''cookieDomain''] respectively.' diff --git a/typo3/sysext/core/Tests/UnitDeprecated/Cache/Backend/PdoBackendTest.php b/typo3/sysext/core/Tests/UnitDeprecated/Cache/Backend/PdoBackendTest.php index 31bba4636a1d..668d6c47c5e7 100644 --- a/typo3/sysext/core/Tests/UnitDeprecated/Cache/Backend/PdoBackendTest.php +++ b/typo3/sysext/core/Tests/UnitDeprecated/Cache/Backend/PdoBackendTest.php @@ -36,6 +36,18 @@ class PdoBackendTest extends UnitTestCase */ protected $resetSingletonInstances = true; + protected function setUp(): void + { + parent::setUp(); + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'secret-encryption-key-test'; + } + + protected function tearDown(): void + { + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']); + parent::tearDown(); + } + /** * @test */ diff --git a/typo3/sysext/frontend/Tests/UnitDeprecated/ContentObject/ContentObjectRendererTest.php b/typo3/sysext/frontend/Tests/UnitDeprecated/ContentObject/ContentObjectRendererTest.php index d74e71906d3c..cc4b1bfdb6a5 100644 --- a/typo3/sysext/frontend/Tests/UnitDeprecated/ContentObject/ContentObjectRendererTest.php +++ b/typo3/sysext/frontend/Tests/UnitDeprecated/ContentObject/ContentObjectRendererTest.php @@ -121,6 +121,8 @@ protected function setUp(): void { parent::setUp(); + $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'secret-encryption-key-test'; + $site = $this->createSiteWithLanguage([ 'base' => '/', 'languageId' => 2, @@ -172,6 +174,12 @@ protected function setUp(): void $this->subject->start([], 'tt_content'); } + protected function tearDown(): void + { + unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']); + parent::tearDown(); + } + /** * @return array */