From df486372ea56fac241d3c96ad43a7729fee64557 Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <ben@bnf.dev>
Date: Tue, 13 Feb 2024 10:06:01 +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/+/82948
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
.../Classes/Configuration/ConfigurationManager.php | 1 +
typo3/sysext/core/Classes/Log/Writer/FileWriter.php | 12 +++++++++++-
.../core/Configuration/DefaultConfiguration.php | 1 -
.../DefaultConfigurationDescription.yaml | 3 ---
.../TypoScript/Parser/TypoScriptParserTest.php | 2 ++
.../Mvc/Web/Routing/UriBuilderTest.php | 7 +++++++
6 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php b/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php
index be665302ded5..8b359c18cdc3 100644
--- a/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php
+++ b/typo3/sysext/core/Classes/Configuration/ConfigurationManager.php
@@ -66,6 +66,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 f01b3a9f21a8..2acf19f06c12 100644
--- a/typo3/sysext/core/Classes/Log/Writer/FileWriter.php
+++ b/typo3/sysext/core/Classes/Log/Writer/FileWriter.php
@@ -66,7 +66,10 @@ class FileWriter extends AbstractWriter
{
// 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());
}
}
@@ -76,6 +79,9 @@ class FileWriter extends AbstractWriter
*/
public function __destruct()
{
+ if ($this->logFile === '') {
+ return;
+ }
self::$logFileHandlesCount[$this->logFile]--;
if (self::$logFileHandlesCount[$this->logFile] <= 0) {
$this->closeLogFile();
@@ -130,6 +136,10 @@ class FileWriter extends AbstractWriter
*/
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 a61c88d7485a..f72582bfd578 100644
--- a/typo3/sysext/core/Configuration/DefaultConfiguration.php
+++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php
@@ -81,7 +81,6 @@ return [
],
'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 e2e5b10d16f5..89b5cca33669 100644
--- a/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
+++ b/typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
@@ -76,9 +76,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. <code>/\.(example1|example2)\.com$/</code> or <code>/\.(example1\.com)|(example2\.net)$/</code>. Separate domains for FE and BE can be set using <a href="#FE-cookieDomain">$TYPO3_CONF_VARS[''FE''][''cookieDomain'']</a> and <a href="#BE-cookieDomain">$TYPO3_CONF_VARS[''BE''][''cookieDomain'']</a> respectively.'
diff --git a/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/Parser/TypoScriptParserTest.php b/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/Parser/TypoScriptParserTest.php
index 0911003ba1c3..c2db2b2fde5e 100644
--- a/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/Parser/TypoScriptParserTest.php
+++ b/typo3/sysext/core/Tests/UnitDeprecated/TypoScript/Parser/TypoScriptParserTest.php
@@ -667,7 +667,9 @@ test.TYPO3Forever.TypoScript = 1
*/
public function importFiles(string $typoScript, string $expected): void
{
+ $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'secret-encryption-key-test';
$resolvedIncludeLines = TypoScriptParser::checkIncludeLines($typoScript);
+ unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']);
self::assertEquals($expected, $resolvedIncludeLines);
}
diff --git a/typo3/sysext/extbase/Tests/UnitDeprecated/Mvc/Web/Routing/UriBuilderTest.php b/typo3/sysext/extbase/Tests/UnitDeprecated/Mvc/Web/Routing/UriBuilderTest.php
index d50addfc1993..efaea1884cba 100644
--- a/typo3/sysext/extbase/Tests/UnitDeprecated/Mvc/Web/Routing/UriBuilderTest.php
+++ b/typo3/sysext/extbase/Tests/UnitDeprecated/Mvc/Web/Routing/UriBuilderTest.php
@@ -37,6 +37,7 @@ final class UriBuilderTest extends UnitTestCase
{
parent::setUp();
$this->mockExtensionService = $this->createMock(ExtensionService::class);
+ $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'] = 'secret-encryption-key-test';
$this->subject = $this->getAccessibleMock(UriBuilder::class, ['build']);
$this->subject->setRequest($this->createMock(Request::class));
$this->subject->injectConfigurationManager($this->createMock(ConfigurationManagerInterface::class));
@@ -44,6 +45,12 @@ final class UriBuilderTest extends UnitTestCase
$this->subject->_set('contentObject', $this->createMock(ContentObjectRenderer::class));
}
+ protected function tearDown(): void
+ {
+ unset($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']);
+ parent::tearDown();
+ }
+
/**
* @test
*/
--
GitLab