[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 <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>

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

@@ -398,7 +398,7 @@ public function resize(string $sourceFile, string $targetFileExtension, int|stri
         }
         // re-apply colorspace-setting for the resulting image so colors don't appear to dark (sRGB instead of RGB)
         if (!str_contains($command, '-colorspace')) {
-            $command .= ' -colorspace ' . $this->colorspace;
+            $command .= ' -colorspace ' . CommandUtility::escapeShellArgument($this->colorspace);
         }
         if ($this->alternativeOutputKey) {
             $theOutputName = md5($command . $processingInstructions->cropArea . PathUtility::basename($sourceFile) . $this->alternativeOutputKey . '[' . $frame . ']');
@@ -651,14 +651,20 @@ public function combineExec($input, $overlay, $mask, $output)
      */
     protected function modifyImageMagickStripProfileParameters(string $parameters, array $options): string
     {
-        if (isset($options['stripProfile'])) {
-            if ($options['stripProfile'] && $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] !== '') {
-                $parameters = $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] . ' ' . $parameters;
-            } else {
-                $parameters .= '###SkipStripProfile###';
-            }
+        if (!isset($options['stripProfile'])) {
+            return $parameters;
         }
-        return $parameters;
+
+        $gfxConf = $GLOBALS['TYPO3_CONF_VARS']['GFX'] ?? [];
+        // Use legacy processor_stripColorProfileCommand setting if defined, otherwise
+        // use the preferred configuration option processor_stripColorProfileParameters
+        $stripColorProfileCommand = $gfxConf['processor_stripColorProfileCommand'] ??
+            implode(' ', array_map(CommandUtility::escapeShellArgument(...), $gfxConf['processor_stripColorProfileParameters'] ?? []));
+        if ($options['stripProfile'] && $stripColorProfileCommand !== '') {
+            return $stripColorProfileCommand . ' ' . $parameters;
+        }
+
+        return $parameters . '###SkipStripProfile###';
     }
 
     /***********************************

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

@@ -27,6 +27,7 @@
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 use TYPO3\CMS\Core\Exception;
 use TYPO3\CMS\Core\Log\LogManagerInterface;
+use TYPO3\CMS\Core\Resource\Security\FileNameValidator;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
@@ -150,6 +151,10 @@ public function get(array $mailSettings): TransportInterface
                 if ($mboxFile === '') {
                     throw new Exception('$GLOBALS[\'TYPO3_CONF_VARS\'][\'MAIL\'][\'transport_mbox_file\'] needs to be set when transport is set to "mbox".', 1294586645);
                 }
+                $fileNameValidator = GeneralUtility::makeInstance(FileNameValidator::class);
+                if (!$fileNameValidator->isValid($mboxFile)) {
+                    throw new Exception('$GLOBALS[\'TYPO3_CONF_VARS\'][\'MAIL\'][\'transport_mbox_file\'] failed against deny-pattern', 1705312431);
+                }
                 // Create our transport
                 $transport = GeneralUtility::makeInstance(
                     MboxTransport::class,

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

@@ -119,14 +119,18 @@ public static function imageMagickCommand(string $command, string $parameters, s
         }
         // strip profile information for thumbnails and reduce their size
         if ($parameters && $command !== 'identify') {
+            // Use legacy processor_stripColorProfileCommand setting if defined, otherwise
+            // use the preferred configuration option processor_stripColorProfileParameters
+            $stripColorProfileCommand = $gfxConf['processor_stripColorProfileCommand'] ??
+                implode(' ', array_map(CommandUtility::escapeShellArgument(...), $gfxConf['processor_stripColorProfileParameters'] ?? []));
             // Determine whether the strip profile action has be disabled by TypoScript:
             if ($gfxConf['processor_stripColorProfileByDefault']
-                && $gfxConf['processor_stripColorProfileCommand'] !== ''
+                && $stripColorProfileCommand !== ''
                 && $parameters !== '-version'
-                && !str_contains($parameters, $gfxConf['processor_stripColorProfileCommand'])
+                && !str_contains($parameters, $stripColorProfileCommand)
                 && !str_contains($parameters, '###SkipStripProfile###')
             ) {
-                $parameters = $gfxConf['processor_stripColorProfileCommand'] . ' ' . $parameters;
+                $parameters = $stripColorProfileCommand . ' ' . $parameters;
             } else {
                 $parameters = str_replace('###SkipStripProfile###', '', $parameters);
             }
@@ -137,7 +141,7 @@ public static function imageMagickCommand(string $command, string $parameters, s
         }
         // set interlace parameter for convert command
         if ($command !== 'identify' && $gfxConf['processor_interlace']) {
-            $parameters = '-interlace ' . $gfxConf['processor_interlace'] . ' ' . $parameters;
+            $parameters = '-interlace ' . CommandUtility::escapeShellArgument($gfxConf['processor_interlace']) . ' ' . $parameters;
         }
         $cmdLine = $path . ' ' . $parameters;
         // It is needed to change the parameters order when a mask image has been specified

typo3/sysext/core/Configuration/DefaultConfiguration.php

@@ -46,7 +46,7 @@
         'processor_allowUpscaling' => true,
         'processor_allowFrameSelection' => true,
         'processor_stripColorProfileByDefault' => true,
-        'processor_stripColorProfileCommand' => '+profile \'*\'',
+        'processor_stripColorProfileParameters' => ['+profile', '*'],
         'processor_colorspace' => '',
         'processor_interlace' => 'None',
         'jpg_quality' => 85,

typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml

@@ -13,6 +13,7 @@ GFX:
             description: 'Enables the use of Image- or GraphicsMagick.'
         processor_path:
             type: text
+            readonly: true
             description: 'Path to the IM tools ''convert'', ''combine'', ''identify''.'
         processor:
             type: dropdown
@@ -31,10 +32,10 @@ GFX:
             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.'
         processor_stripColorProfileByDefault:
             type: bool
-            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.'
-        processor_stripColorProfileCommand:
-            type: text
-            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'
+            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.'
+        processor_stripColorProfileParameters:
+            type: array
+            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'
         processor_colorspace:
             type: text
             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:
             description: 'Content-Security-Policy reporting HTTP endpoint, if empty system default will be used'
         fileDenyPattern:
             type: text
+            readonly: true
             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'
         versionNumberInFilename:
             type: bool
@@ -598,6 +600,7 @@ MAIL:
         transport_sendmail_command:
             type: text
             description: '<em>only with transport=sendmail</em>: The command to call to send a mail locally.'
+            readonly: true
         transport_mbox_file:
             type: text
             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

@@ -0,0 +1,40 @@
+.. include:: /Includes.rst.txt
+
+.. _important-102799-1707403491:
+
+===========================================================================================
+Important: #102799 - TYPO3_CONF_VARS.GFX.processor_stripColorProfileParameters option added
+===========================================================================================
+
+See :issue:`102799`
+
+Description
+===========
+
+The string-based configuration option
+:php:`$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand']`
+has been superseded by
+:php:`$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters']`
+for security reasons.
+
+The former option expected a string of command line parameters. The defined
+parameters had to be shell-escaped beforehand, while the new option expects an
+array of strings that will be shell-escaped by TYPO3 when used.
+
+The existing configuration will continue to be supported. Still, it is suggested
+to use the new configuration format, as the Install Tool is adapted to allow
+modification of the new configuration option only:
+
+..  code-block:: php
+
+    // Before
+    $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand'] = '+profile \'*\'';
+
+    // After
+    $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters'] = [
+        '+profile',
+        '*'
+    ];
+
+
+.. index:: LocalConfiguration, ext:core

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

@@ -3627,7 +3627,7 @@ public function getImgResource($file, $fileArray)
                 }
 
                 // Possibility to cancel/force profile extraction
-                // see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileCommand']
+                // see $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_stripColorProfileParameters']
                 if (isset($fileArray['stripProfile'])) {
                     $processingConfiguration['stripProfile'] = $fileArray['stripProfile'];
                 }

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

@@ -70,25 +70,40 @@ public function isAvailable()
     }
 
     /**
-     * Get configuration values is used in fluid to show configuration options.
+     * Get configuration values is used to persist data and is merged with given $postValues.
+     *
+     * @return array Configuration values needed to activate prefix
+     */
+    public function getConfigurationValues()
+    {
+        return array_map(static fn($configuration) => $configuration['value'], $this->getConfigurationDescriptors());
+    }
+
+    /**
+     * Build configuration descriptors to be used in fluid to show configuration options.
      * They are fetched from LocalConfiguration / DefaultConfiguration and
      * merged with given $postValues.
      *
      * @return array Configuration values needed to activate prefix
      */
-    public function getConfigurationValues()
+    public function getConfigurationDescriptors()
     {
         $configurationValues = [];
         foreach ($this->configurationValues as $configurationKey => $configurationValue) {
-            if (isset($this->postValues['enable'])
+            $readonly = isset($this->readonlyConfigurationValues[$configurationKey]);
+            if (!$readonly
+                && isset($this->postValues['enable'])
                 && $this->postValues['enable'] === $this->name
                 && isset($this->postValues[$this->name][$configurationKey])
             ) {
                 $currentValue = $this->postValues[$this->name][$configurationKey];
             } else {
                 $currentValue = $this->configurationManager->getConfigurationValueByPath($configurationKey);
             }
-            $configurationValues[$configurationKey] = $currentValue;
+            $configurationValues[$configurationKey] = [
+                'value' => $currentValue,
+                'readonly' => $readonly,
+            ];
         }
         return $configurationValues;
     }

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

@@ -45,6 +45,11 @@ abstract class AbstractPreset implements PresetInterface
      */
     protected $configurationValues = [];
 
+    /**
+     * @var array Configuration values that are visible but not editable via presets GUI
+     */
+    protected $readonlyConfigurationValues = [];
+
     /**
      * @var array List of $POST values
      */

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

@@ -34,4 +34,8 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
         'GFX/processor_effects' => false,
         'GFX/processor_colorspace' => '',
     ];
+
+    protected $readonlyConfigurationValues = [
+        'GFX/processor_path' => true,
+    ];
 }

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

@@ -35,4 +35,8 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
         'MAIL/transport_smtp_username' => '',
         'MAIL/transport_smtp_password' => '',
     ];
+
+    protected $readonlyConfigurationValues = [
+        'MAIL/transport_sendmail_command' => true,
+    ];
 }

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

@@ -32,22 +32,35 @@ class CustomPreset extends AbstractCustomPreset implements CustomPresetInterface
      * Get configuration values is used in fluid to show configuration options.
      * They are fetched from LocalConfiguration / DefaultConfiguration.
      *
+     * They are not merged with postValues for security reasons, as
+     * all options are readonly.
+     *
      * @return array Current custom configuration values
      */
-    public function getConfigurationValues(): array
+    public function getConfigurationDescriptors(): array
     {
         $configurationValues = [];
-        $configurationValues['BE/passwordHashing/className'] =
-            $this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/className');
+        $configurationValues['BE/passwordHashing/className'] = [
+            'value' => $this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/className'),
+            'readonly' => true,
+        ];
         $options = (array)$this->configurationManager->getConfigurationValueByPath('BE/passwordHashing/options');
         foreach ($options as $optionName => $optionValue) {
-            $configurationValues['BE/passwordHashing/options/' . $optionName] = $optionValue;
+            $configurationValues['BE/passwordHashing/options/' . $optionName] = [
+                'value' => $optionValue,
+                'readonly' => true,
+            ];
         }
-        $configurationValues['FE/passwordHashing/className'] =
-            $this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/className');
+        $configurationValues['FE/passwordHashing/className'] = [
+            'value' => $this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/className'),
+            'readonly' => true,
+        ];
         $options = (array)$this->configurationManager->getConfigurationValueByPath('FE/passwordHashing/options');
         foreach ($options as $optionName => $optionValue) {
-            $configurationValues['FE/passwordHashing/options/' . $optionName] = $optionValue;
+            $configurationValues['FE/passwordHashing/options/' . $optionName] = [
+                'value' => $optionValue,
+                'readonly' => true,
+            ];
         }
         return $configurationValues;
     }

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

@@ -21,6 +21,8 @@
 use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
 use TYPO3\CMS\Core\Messaging\FlashMessage;
 use TYPO3\CMS\Core\Messaging\FlashMessageQueue;
+use TYPO3\CMS\Core\Type\ContextualFeedbackSeverity;
+use TYPO3\CMS\Core\Utility\Exception\MissingArrayPathException;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
@@ -90,6 +92,7 @@ protected function recursiveConfigurationFetching(array $sections, array $sectio
                 $itemData['path'] = '[' . implode('][', $newPath) . ']';
                 $itemData['fieldType'] = $descriptionInfo['type'];
                 $itemData['description'] = $descriptionInfo['description'] ?? '';
+                $itemData['readonly'] = $descriptionInfo['readonly'] ?? false;
                 $itemData['allowedValues'] = $descriptionInfo['allowedValues'] ?? [];
                 $itemData['differentValueInCurrentConfiguration'] = (!isset($descriptionInfo['compareValuesWithCurrentConfiguration']) ||
                     $descriptionInfo['compareValuesWithCurrentConfiguration']) &&
@@ -151,11 +154,28 @@ public function updateLocalConfigurationValues(array $valueList): FlashMessageQu
         $commentArray = $this->getDefaultConfigArrayComments();
         $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
         foreach ($valueList as $path => $value) {
-            $oldValue = $configurationManager->getConfigurationValueByPath($path);
+            try {
+                $oldValue = $configurationManager->getConfigurationValueByPath($path);
+            } catch (MissingArrayPathException) {
+                $messageQueue->enqueue(new FlashMessage(
+                    'Update rejected, the category of this setting does not exist',
+                    $path,
+                    ContextualFeedbackSeverity::ERROR
+                ));
+                continue;
+            }
             $pathParts = explode('/', $path);
             $descriptionData = $commentArray[$pathParts[0]];
 
             while ($part = next($pathParts)) {
+                if (!isset($descriptionData['items'][$part])) {
+                    $messageQueue->enqueue(new FlashMessage(
+                        'Update rejected, this setting is not writable',
+                        $path,
+                        ContextualFeedbackSeverity::ERROR
+                    ));
+                    continue 2;
+                }
                 $descriptionData = $descriptionData['items'][$part];
             }
 
@@ -183,6 +203,16 @@ public function updateLocalConfigurationValues(array $valueList): FlashMessageQu
                 $valueHasChanged = (string)$oldValue !== (string)$value;
             }
 
+            $readonly = $descriptionData['readonly'] ?? false;
+            if ($readonly && $valueHasChanged) {
+                $messageQueue->enqueue(new FlashMessage(
+                    'Update rejected, this setting is readonly',
+                    $path,
+                    ContextualFeedbackSeverity::ERROR
+                ));
+                continue;
+            }
+
             // Save if value changed
             if ($valueHasChanged) {
                 $configurationPathValuePairs[$path] = $value;