Skip to content

Commit 6cc1176

Browse files
bnfohader
bnf
authored and
ohader
committed
[SECURITY] Prevent RCE via install tool settings
Resolves: #102799 Releases: main, 13.0, 12.4, 11.5 Change-Id: I673b6fbac853b0a977a5e5833a683c6952a55458 Security-Bulletin: TYPO3-CORE-SA-2024-002 Security-References: CVE-2024-22188 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82946 Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>
1 parent 78fb928 commit 6cc1176

File tree

21 files changed

+200
-55
lines changed

21 files changed

+200
-55
lines changed

‎typo3/sysext/core/Classes/Imaging/GraphicalFunctions.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2160,7 +2160,7 @@ public function imageMagickConvert($imagefile, $newExt = '', $w = '', $h = '', $
21602160
}
21612161
$command .= ' ' . $info[0] . 'x' . $info[1] . '! ' . $params . ' ';
21622162
// re-apply colorspace-setting for the resulting image so colors don't appear to dark (sRGB instead of RGB)
2163-
$command .= ' -colorspace ' . $this->colorspace;
2163+
$command .= ' -colorspace ' . CommandUtility::escapeShellArgument($this->colorspace);
21642164
$cropscale = $data['crs'] ? 'crs-V' . $data['cropV'] . 'H' . $data['cropH'] : '';
21652165
if ($this->alternativeOutputKey) {
21662166
$theOutputName = md5($command . $cropscale . PathUtility::basename($imagefile) . $this->alternativeOutputKey . '[' . $frame . ']');

‎typo3/sysext/core/Classes/Mail/TransportFactory.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
2828
use TYPO3\CMS\Core\Exception;
2929
use TYPO3\CMS\Core\Log\LogManagerInterface;
30+
use TYPO3\CMS\Core\Resource\Security\FileNameValidator;
3031
use TYPO3\CMS\Core\SingletonInterface;
3132
use TYPO3\CMS\Core\Utility\GeneralUtility;
3233

@@ -150,6 +151,10 @@ public function get(array $mailSettings): TransportInterface
150151
if ($mboxFile === '') {
151152
throw new Exception('$GLOBALS[\'TYPO3_CONF_VARS\'][\'MAIL\'][\'transport_mbox_file\'] needs to be set when transport is set to "mbox".', 1294586645);
152153
}
154+
$fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
155+
if (!$fileNameValidator->isValid($mboxFile)) {
156+
throw new Exception('$GLOBALS[\'TYPO3_CONF_VARS\'][\'MAIL\'][\'transport_mbox_file\'] failed against deny-pattern', 1705312431);
157+
}
153158
// Create our transport
154159
$transport = GeneralUtility::makeInstance(
155160
MboxTransport::class,

‎typo3/sysext/core/Classes/Resource/Processing/LocalCropScaleMaskHelper.php

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use TYPO3\CMS\Core\Imaging\GraphicalFunctions;
2020
use TYPO3\CMS\Core\Resource\FileInterface;
2121
use TYPO3\CMS\Core\Resource\ProcessedFile;
22+
use TYPO3\CMS\Core\Utility\CommandUtility;
2223
use TYPO3\CMS\Core\Utility\GeneralUtility;
2324
use TYPO3\CMS\Core\Utility\MathUtility;
2425
use TYPO3\CMS\Frontend\Imaging\GifBuilder;
@@ -286,18 +287,21 @@ protected function getFilenameForImageCropScaleMask(TaskInterface $task)
286287
*/
287288
protected function modifyImageMagickStripProfileParameters(string $parameters, array $configuration)
288289
{
290+
if (!isset($configuration['stripProfile'])) {
291+
return $parameters;
292+
}
293+
294+
$gfxConf = $GLOBALS['TYPO3_CONF_VARS']['GFX'] ?? [];
295+
// Use legacy processor_stripColorProfileCommand setting if defined, otherwise
296+
// use the preferred configuration option processor_stripColorProfileParameters
297+
$stripColorProfileCommand = $gfxConf['processor_stripColorProfileCommand'] ??
298+
implode(' ', array_map(CommandUtility::escapeShellArgument(...), $gfxConf['processor_stripColorProfileParameters'] ?? []));
299+
289300
// Strips profile information of image to save some space:
290-
if (isset($configuration['stripProfile'])) {
291-
if (
292-
$configuration['stripProfile']
293-
&& $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] !== ''
294-
) {
295-
$parameters = $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] . $parameters;
296-
} else {
297-
$parameters .= '###SkipStripProfile###';
298-
}
301+
if ($configuration['stripProfile'] && $stripColorProfileCommand !== '') {
302+
return $stripColorProfileCommand . $parameters;
299303
}
300-
return $parameters;
304+
return $parameters . '###SkipStripProfile###';
301305
}
302306

303307
protected function isTemporaryFile(string $filePath): bool

‎typo3/sysext/core/Classes/Utility/CommandUtility.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,18 @@ public static function imageMagickCommand(string $command, string $parameters, s
119119
}
120120
// strip profile information for thumbnails and reduce their size
121121
if ($parameters && $command !== 'identify') {
122+
// Use legacy processor_stripColorProfileCommand setting if defined, otherwise
123+
// use the preferred configuration option processor_stripColorProfileParameters
124+
$stripColorProfileCommand = $gfxConf['processor_stripColorProfileCommand'] ??
125+
implode(' ', array_map(CommandUtility::escapeShellArgument(...), $gfxConf['processor_stripColorProfileParameters'] ?? []));
122126
// Determine whether the strip profile action has be disabled by TypoScript:
123127
if ($gfxConf['processor_stripColorProfileByDefault']
124-
&& $gfxConf['processor_stripColorProfileCommand'] !== ''
128+
&& $stripColorProfileCommand !== ''
125129
&& $parameters !== '-version'
126-
&& !str_contains($parameters, $gfxConf['processor_stripColorProfileCommand'])
130+
&& !str_contains($parameters, $stripColorProfileCommand)
127131
&& !str_contains($parameters, '###SkipStripProfile###')
128132
) {
129-
$parameters = $gfxConf['processor_stripColorProfileCommand'] . ' ' . $parameters;
133+
$parameters = $stripColorProfileCommand . ' ' . $parameters;
130134
} else {
131135
$parameters = str_replace('###SkipStripProfile###', '', $parameters);
132136
}
@@ -137,7 +141,7 @@ public static function imageMagickCommand(string $command, string $parameters, s
137141
}
138142
// set interlace parameter for convert command
139143
if ($command !== 'identify' && $gfxConf['processor_interlace']) {
140-
$parameters = '-interlace ' . $gfxConf['processor_interlace'] . ' ' . $parameters;
144+
$parameters = '-interlace ' . CommandUtility::escapeShellArgument($gfxConf['processor_interlace']) . ' ' . $parameters;
141145
}
142146
$cmdLine = $path . ' ' . $parameters;
143147
// It is needed to change the parameters order when a mask image has been specified

‎typo3/sysext/core/Configuration/DefaultConfiguration.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
'processor_allowFrameSelection' => true,
3838
'processor_allowTemporaryMasksAsPng' => false,
3939
'processor_stripColorProfileByDefault' => true,
40-
'processor_stripColorProfileCommand' => '+profile \'*\'',
40+
'processor_stripColorProfileParameters' => ['+profile', '*'],
4141
'processor_colorspace' => 'RGB',
4242
'processor_interlace' => 'None',
4343
'jpg_quality' => 85,

‎typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ GFX:
2525
description: 'Enables the use of Image- or GraphicsMagick.'
2626
processor_path:
2727
type: text
28+
readonly: true
2829
description: 'Path to the IM tools ''convert'', ''combine'', ''identify''.'
2930
processor:
3031
type: dropdown
@@ -46,10 +47,10 @@ GFX:
4647
description: 'This should be set if your processor supports using PNGs as masks as this is usually faster.'
4748
processor_stripColorProfileByDefault:
4849
type: bool
49-
description: 'If set, the processor_stripColorProfileCommand is used with all processor image operations by default. See tsRef for setting this parameter explicitly for IMAGE generation.'
50-
processor_stripColorProfileCommand:
51-
type: text
52-
description: 'String: Specify the command to strip the profile information, which can reduce thumbnail size up to 60KB. Command can differ in IM/GM, IM also know the -strip command. See <a href="http://www.imagemagick.org/Usage/thumbnails/#profiles" target="_blank" rel="noreferrer">imagemagick.org</a> for details'
50+
description: 'If set, the processor_stripColorProfileParameters is used with all processor image operations by default. See tsRef for setting this parameter explicitly for IMAGE generation.'
51+
processor_stripColorProfileParameters:
52+
type: array
53+
description: 'Comma separated list of parameters: Specify the parameters to strip the profile information, which can reduce thumbnail size up to 60KB. Command can differ in IM/GM, IM also know the -strip command. See <a href="http://www.imagemagick.org/Usage/thumbnails/#profiles" target="_blank" rel="noreferrer">imagemagick.org</a> for details'
5354
processor_colorspace:
5455
type: text
5556
description: 'String: Specify the colorspace to use. Some ImageMagick versions (like 6.7.0 and above) use the sRGB colorspace, so all images are darker then the original. <br />Possible Values: CMY, CMYK, Gray, HCL, HSB, HSL, HWB, Lab, LCH, LMS, Log, Luv, OHTA, Rec601Luma, Rec601YCbCr, Rec709Luma, Rec709YCbCr, RGB, sRGB, Transparent, XYZ, YCbCr, YCC, YIQ, YCbCr, YUV'
@@ -377,6 +378,7 @@ BE:
377378
description: 'Content-Security-Policy reporting HTTP endpoint, if empty system default will be used'
378379
fileDenyPattern:
379380
type: text
381+
readonly: true
380382
description: 'A perl-compatible and JavaScript-compatible regular expression (without delimiters "/"!) that - if it matches a filename - will deny the file upload/rename or whatever. For security reasons, files with multiple extensions have to be denied on an Apache environment with mod_alias, if the filename contains a valid php handler in an arbitrary position. Also, ".htaccess" files have to be denied. Matching is done case-insensitive. Default value is stored in PHP constant FILE_DENY_PATTERN_DEFAULT'
381383
flexformForceCDATA:
382384
type: bool
@@ -627,6 +629,7 @@ MAIL:
627629
transport_sendmail_command:
628630
type: text
629631
description: '<em>only with transport=sendmail</em>: The command to call to send a mail locally.'
632+
readonly: true
630633
transport_mbox_file:
631634
type: text
632635
description: '<em>only with transport=mbox</em>: The file where to write the mails into. This file will be conforming the mbox format described in RFC 4155. It is a simple text file with a concatenation of all mails. Path must be absolute.'

‎typo3/sysext/core/Documentation/Changelog/11.5.x/Important-102799-TYPO3_CONF_VARSGFXprocessor_stripColorProfileParametersOptionAdded.rst

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
.. include:: /Includes.rst.txt
2+
3+
.. _important-102799-1707403491:
4+
5+
===========================================================================================
6+
Important: #102799 - TYPO3_CONF_VARS.GFX.processor_stripColorProfileParameters option added
7+
===========================================================================================
8+
9+
See :issue:`102799`
10+
11+
Description
12+
===========
13+
14+
The string-based configuration option
15+
:php:`$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand']`
16+
has been superseded by
17+
:php:`$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters']`
18+
for security reasons.
19+
20+
The former option expected a string of command line parameters. The defined
21+
parameters had to be shell-escaped beforehand, while the new option expects an
22+
array of strings that will be shell-escaped by TYPO3 when used.
23+
24+
The existing configuration will continue to be supported. Still, it is suggested
25+
to use the new configuration format, as the Install Tool is adapted to allow
26+
modification of the new configuration option only:
27+
28+
.. code-block:: php
29+
30+
// Before
31+
$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] = '+profile \'*\'';
32+
33+
// After
34+
$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters'] = [
35+
'+profile',
36+
'*'
37+
];
38+
39+
40+
.. index:: LocalConfiguration, ext:core

‎typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3821,7 +3821,7 @@ public function getImgResource($file, $fileArray)
38213821
}
38223822

38233823
// Possibility to cancel/force profile extraction
3824-
// see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand']
3824+
// see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters']
38253825
if (isset($fileArray['stripProfile'])) {
38263826
$processingConfiguration['stripProfile'] = $fileArray['stripProfile'];
38273827
}

‎typo3/sysext/install/Classes/Configuration/AbstractCustomPreset.php

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,25 +70,40 @@ public function isAvailable()
7070
}
7171

7272
/**
73-
* Get configuration values is used in fluid to show configuration options.
73+
* Get configuration values is used to persist data and is merged with given $postValues.
74+
*
75+
* @return array Configuration values needed to activate prefix
76+
*/
77+
public function getConfigurationValues()
78+
{
79+
return array_map(static fn($configuration) => $configuration['value'], $this->getConfigurationDescriptors());
80+
}
81+
82+
/**
83+
* Build configuration descriptors to be used in fluid to show configuration options.
7484
* They are fetched from LocalConfiguration / DefaultConfiguration and
7585
* merged with given $postValues.
7686
*
7787
* @return array Configuration values needed to activate prefix
7888
*/
79-
public function getConfigurationValues()
89+
public function getConfigurationDescriptors()
8090
{
8191
$configurationValues = [];
8292
foreach ($this->configurationValues as $configurationKey => $configurationValue) {
83-
if (isset($this->postValues['enable'])
93+
$readonly = isset($this->readonlyConfigurationValues[$configurationKey]);
94+
if (!$readonly
95+
&& isset($this->postValues['enable'])
8496
&& $this->postValues['enable'] === $this->name
8597
&& isset($this->postValues[$this->name][$configurationKey])
8698
) {
8799
$currentValue = $this->postValues[$this->name][$configurationKey];
88100
} else {
89101
$currentValue = $this->configurationManager->getConfigurationValueByPath($configurationKey);
90102
}
91-
$configurationValues[$configurationKey] = $currentValue;
103+
$configurationValues[$configurationKey] = [
104+
'value' => $currentValue,
105+
'readonly' => $readonly,
106+
];
92107
}
93108
return $configurationValues;
94109
}

‎typo3/sysext/install/Classes/Configuration/AbstractPreset.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ abstract class AbstractPreset implements PresetInterface
4545
*/
4646
protected $configurationValues = [];
4747

48+
/**
49+
* @var array Configuration values that are visible but not editable via presets GUI
50+
*/
51+
protected $readonlyConfigurationValues = [];
52+
4853
/**
4954
* @var array List of $POST values
5055
*/

‎typo3/sysext/install/Classes/Configuration/Image/CustomPreset.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,8 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
3535
'GFX/processor_allowTemporaryMasksAsPng' => true,
3636
'GFX/processor_colorspace' => '',
3737
];
38+
39+
protected $readonlyConfigurationValues = [
40+
'GFX/processor_path' => true,
41+
];
3842
}

‎typo3/sysext/install/Classes/Configuration/Mail/CustomPreset.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,8 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
3535
'MAIL/transport_smtp_username' => '',
3636
'MAIL/transport_smtp_password' => '',
3737
];
38+
39+
protected $readonlyConfigurationValues = [
40+
'MAIL/transport_sendmail_command' => true,
41+
];
3842
}

‎typo3/sysext/install/Classes/Configuration/PasswordHashing/CustomPreset.php

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,35 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
3232
* Get configuration values is used in fluid to show configuration options.
3333
* They are fetched from LocalConfiguration / DefaultConfiguration.
3434
*
35+
* They are not merged with postValues for security reasons, as
36+
* all options are readonly.
37+
*
3538
* @return array Current custom configuration values
3639
*/
37-
public function getConfigurationValues(): array
40+
public function getConfigurationDescriptors(): array
3841
{
3942
$configurationValues = [];
40-
$configurationValues['BE/passwordHashing/className'] =
41-
$this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/className');
43+
$configurationValues['BE/passwordHashing/className'] = [
44+
'value' => $this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/className'),
45+
'readonly' => true,
46+
];
4247
$options = (array)$this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/options');
4348
foreach ($options as $optionName => $optionValue) {
44-
$configurationValues['BE/passwordHashing/options/' . $optionName] = $optionValue;
49+
$configurationValues['BE/passwordHashing/options/' . $optionName] = [
50+
'value' => $optionValue,
51+
'readonly' => true,
52+
];
4553
}
46-
$configurationValues['FE/passwordHashing/className'] =
47-
$this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/className');
54+
$configurationValues['FE/passwordHashing/className'] = [
55+
'value' => $this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/className'),
56+
'readonly' => true,
57+
];
4858
$options = (array)$this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/options');
4959
foreach ($options as $optionName => $optionValue) {
50-
$configurationValues['FE/passwordHashing/options/' . $optionName] = $optionValue;
60+
$configurationValues['FE/passwordHashing/options/' . $optionName] = [
61+
'value' => $optionValue,
62+
'readonly' => true,
63+
];
5164
}
5265
return $configurationValues;
5366
}

‎typo3/sysext/install/Classes/Service/LocalConfigurationValueService.php

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
2222
use TYPO3\CMS\Core\Messaging\FlashMessage;
2323
use TYPO3\CMS\Core\Messaging\FlashMessageQueue;
24+
use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity;
25+
use TYPO3\CMS\Core\Utility\Exception\MissingArrayPathException;
2426
use TYPO3\CMS\Core\Utility\GeneralUtility;
2527

2628
/**
@@ -90,6 +92,7 @@ protected function recursiveConfigurationFetching(array $sections, array $sectio
9092
$itemData['path'] = '[' . implode('][', $newPath) . ']';
9193
$itemData['fieldType'] = $descriptionInfo['type'];
9294
$itemData['description'] = $descriptionInfo['description'] ?? '';
95+
$itemData['readonly'] = $descriptionInfo['readonly'] ?? false;
9396
$itemData['allowedValues'] = $descriptionInfo['allowedValues'] ?? [];
9497
$itemData['differentValueInCurrentConfiguration'] = (!isset($descriptionInfo['compareValuesWithCurrentConfiguration']) ||
9598
$descriptionInfo['compareValuesWithCurrentConfiguration']) &&
@@ -151,11 +154,28 @@ public function updateLocalConfigurationValues(array $valueList): FlashMessageQu
151154
$commentArray = $this->getDefaultConfigArrayComments();
152155
$configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
153156
foreach ($valueList as $path => $value) {
154-
$oldValue = $configurationManager->getConfigurationValueByPath($path);
157+
try {
158+
$oldValue = $configurationManager->getConfigurationValueByPath($path);
159+
} catch (MissingArrayPathException) {
160+
$messageQueue->enqueue(new FlashMessage(
161+
'Update rejected, the category of this setting does not exist',
162+
$path,
163+
ContextualFeedbackSeverity::ERROR
164+
));
165+
continue;
166+
}
155167
$pathParts = explode('/', $path);
156168
$descriptionData = $commentArray[$pathParts[0]];
157169

158170
while ($part = next($pathParts)) {
171+
if (!isset($descriptionData['items'][$part])) {
172+
$messageQueue->enqueue(new FlashMessage(
173+
'Update rejected, this setting is not writable',
174+
$path,
175+
ContextualFeedbackSeverity::ERROR
176+
));
177+
continue 2;
178+
}
159179
$descriptionData = $descriptionData['items'][$part];
160180
}
161181

@@ -183,6 +203,16 @@ public function updateLocalConfigurationValues(array $valueList): FlashMessageQu
183203
$valueHasChanged = (string)$oldValue !== (string)$value;
184204
}
185205

206+
$readonly = $descriptionData['readonly'] ?? false;
207+
if ($readonly && $valueHasChanged) {
208+
$messageQueue->enqueue(new FlashMessage(
209+
'Update rejected, this setting is readonly',
210+
$path,
211+
ContextualFeedbackSeverity::ERROR
212+
));
213+
continue;
214+
}
215+
186216
// Save if value changed
187217
if ($valueHasChanged) {
188218
$configurationPathValuePairs[$path] = $value;

0 commit comments

Comments
 (0)