Skip to content

Commit 47e897f

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/+/82952 Reviewed-by: Oliver Hader <[email protected]> Tested-by: Oliver Hader <[email protected]>
1 parent 205115c commit 47e897f

File tree

20 files changed

+199
-52
lines changed

20 files changed

+199
-52
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ public function resize(string $sourceFile, string $targetFileExtension, int|stri
398398
}
399399
// re-apply colorspace-setting for the resulting image so colors don't appear to dark (sRGB instead of RGB)
400400
if (!str_contains($command, '-colorspace')) {
401-
$command .= ' -colorspace ' . $this->colorspace;
401+
$command .= ' -colorspace ' . CommandUtility::escapeShellArgument($this->colorspace);
402402
}
403403
if ($this->alternativeOutputKey) {
404404
$theOutputName = md5($command . $processingInstructions->cropArea . PathUtility::basename($sourceFile) . $this->alternativeOutputKey . '[' . $frame . ']');
@@ -651,14 +651,20 @@ public function combineExec($input, $overlay, $mask, $output)
651651
*/
652652
protected function modifyImageMagickStripProfileParameters(string $parameters, array $options): string
653653
{
654-
if (isset($options['stripProfile'])) {
655-
if ($options['stripProfile'] && $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] !== '') {
656-
$parameters = $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] . ' ' . $parameters;
657-
} else {
658-
$parameters .= '###SkipStripProfile###';
659-
}
654+
if (!isset($options['stripProfile'])) {
655+
return $parameters;
660656
}
661-
return $parameters;
657+
658+
$gfxConf = $GLOBALS['TYPO3_CONF_VARS']['GFX'] ?? [];
659+
// Use legacy processor_stripColorProfileCommand setting if defined, otherwise
660+
// use the preferred configuration option processor_stripColorProfileParameters
661+
$stripColorProfileCommand = $gfxConf['processor_stripColorProfileCommand'] ??
662+
implode(' ', array_map(CommandUtility::escapeShellArgument(...), $gfxConf['processor_stripColorProfileParameters'] ?? []));
663+
if ($options['stripProfile'] && $stripColorProfileCommand !== '') {
664+
return $stripColorProfileCommand . ' ' . $parameters;
665+
}
666+
667+
return $parameters . '###SkipStripProfile###';
662668
}
663669

664670
/***********************************

‎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/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
@@ -46,7 +46,7 @@
4646
'processor_allowUpscaling' => true,
4747
'processor_allowFrameSelection' => true,
4848
'processor_stripColorProfileByDefault' => true,
49-
'processor_stripColorProfileCommand' => '+profile \'*\'',
49+
'processor_stripColorProfileParameters' => ['+profile', '*'],
5050
'processor_colorspace' => '',
5151
'processor_interlace' => 'None',
5252
'jpg_quality' => 85,

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ GFX:
1313
description: 'Enables the use of Image- or GraphicsMagick.'
1414
processor_path:
1515
type: text
16+
readonly: true
1617
description: 'Path to the IM tools ''convert'', ''combine'', ''identify''.'
1718
processor:
1819
type: dropdown
@@ -31,10 +32,10 @@ GFX:
3132
description: 'If set, the [x] frame selector is appended to input filenames in stdgraphic. This speeds up image processing for PDF files considerably. Disable if your image processor or environment can''t cope with the frame selection.'
3233
processor_stripColorProfileByDefault:
3334
type: bool
34-
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.'
35-
processor_stripColorProfileCommand:
36-
type: text
37-
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'
35+
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.'
36+
processor_stripColorProfileParameters:
37+
type: array
38+
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'
3839
processor_colorspace:
3940
type: text
4041
description: 'String: Specify the colorspace to use. Defaults to "RGB" when using GraphicsMagick as processor and "sRGB" when using ImageMagick. Images would be rendered darker than the original when using ImageMagick in combination with "RGB". <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'
@@ -351,6 +352,7 @@ BE:
351352
description: 'Content-Security-Policy reporting HTTP endpoint, if empty system default will be used'
352353
fileDenyPattern:
353354
type: text
355+
readonly: true
354356
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'
355357
versionNumberInFilename:
356358
type: bool
@@ -598,6 +600,7 @@ MAIL:
598600
transport_sendmail_command:
599601
type: text
600602
description: '<em>only with transport=sendmail</em>: The command to call to send a mail locally.'
603+
readonly: true
601604
transport_mbox_file:
602605
type: text
603606
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
@@ -3627,7 +3627,7 @@ public function getImgResource($file, $fileArray)
36273627
}
36283628

36293629
// Possibility to cancel/force profile extraction
3630-
// see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand']
3630+
// see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters']
36313631
if (isset($fileArray['stripProfile'])) {
36323632
$processingConfiguration['stripProfile'] = $fileArray['stripProfile'];
36333633
}

‎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
@@ -34,4 +34,8 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
3434
'GFX/processor_effects' => false,
3535
'GFX/processor_colorspace' => '',
3636
];
37+
38+
protected $readonlyConfigurationValues = [
39+
'GFX/processor_path' => true,
40+
];
3741
}

‎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)