[BUGFIX] Harden CommandUtility invocations 48/59448/2
authorOliver Hader <oliver@typo3.org>
Mon, 10 Dec 2018 07:51:21 +0000 (08:51 +0100)
committerOliver Hader <oliver.hader@typo3.org>
Thu, 17 Jan 2019 15:14:18 +0000 (16:14 +0100)
In order to harden CommandUtility API arguments used for invoking
system commands are escaped in addition. Since no insecure usages
have been identified in the TYPO3 core nor in public third party
extensions, this change is handled using a public workflow.

| In order to evaluate whether third party extensions open a
| potential attack vector, usages of CommandUtility::checkCommand(),
| CommandUtility::getCommand() and the registration of custom services
| ($GLOBALS[‘T3_SERVICES’]) concerning their ‘exec’ argument have to
| be checked.

Resolves: #87450
Releases: master, 9.5, 8.7
Security-Advisory: TYPO3-PSA-2019-001
Change-Id: If4f2a63045ac7b2473881992f9731a635a768d37
Reviewed-on: https://review.typo3.org/59448
Tested-by: TYPO3com <noreply@typo3.com>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Classes/Imaging/GraphicalFunctions.php
typo3/sysext/core/Classes/Resource/Processing/LocalPreviewHelper.php
typo3/sysext/core/Classes/Utility/CommandUtility.php

index 715312b..4b7bd33 100644 (file)
@@ -2527,7 +2527,11 @@ class GraphicalFunctions
             $temporaryName = PathUtility::dirname($theFile) . '/' . md5(uniqid('', true)) . '.gif';
             // Rename could fail, if a simultaneous thread is currently working on the same thing
             if (@rename($theFile, $temporaryName)) {
-                $cmd = CommandUtility::imageMagickCommand('convert', '"' . $temporaryName . '" "' . $theFile . '"', $gfxConf['processor_path_lzw']);
+                $cmd = CommandUtility::imageMagickCommand(
+                    'convert',
+                    implode(' ', CommandUtility::escapeShellArguments([$temporaryName, $theFile])),
+                    $gfxConf['processor_path_lzw']
+                );
                 CommandUtility::exec($cmd);
                 unlink($temporaryName);
             }
@@ -2575,7 +2579,7 @@ class GraphicalFunctions
         $newFile = Environment::getPublicPath() . '/typo3temp/assets/images/' . md5($theFile . '|' . filemtime($theFile)) . ($output_png ? '.png' : '.gif');
         $cmd = CommandUtility::imageMagickCommand(
             'convert',
-            '"' . $theFile . '" "' . $newFile . '"',
+            implode(' ', CommandUtility::escapeShellArguments([$theFile, $newFile])),
             $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path']
         );
         CommandUtility::exec($cmd);
index f14bf6e..e395302 100644 (file)
@@ -148,8 +148,14 @@ class LocalPreviewHelper
         } else {
             // Create the temporary file
             if ($GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_enabled']) {
-                $parameters = '-sample ' . $configuration['width'] . 'x' . $configuration['height'] . ' '
-                    . CommandUtility::escapeShellArgument($originalFileName) . '[0] ' . CommandUtility::escapeShellArgument($targetFilePath);
+                $arguments = CommandUtility::escapeShellArguments([
+                    'width' => $configuration['width'],
+                    'height' => $configuration['height'],
+                    'originalFileName' => $originalFileName,
+                    'targetFilePath' => $targetFilePath,
+                ]);
+                $parameters = '-sample ' . $arguments['width'] . 'x' . $arguments['height'] . ' '
+                    . $arguments['originalFileName'] . '[0] ' . $arguments['targetFilePath'];
 
                 $cmd = CommandUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
                 CommandUtility::exec($cmd);
index a8b2e04..205b7ed 100644 (file)
@@ -211,7 +211,7 @@ class CommandUtility
         // Try to get the executable with the command 'which'.
         // It does the same like already done, but maybe on other paths
         if (!Environment::isWindows()) {
-            $cmd = @self::exec('which ' . $cmd);
+            $cmd = @self::exec('which ' . self::escapeShellArgument($cmd));
             if (@is_executable($cmd)) {
                 self::$applications[$cmd]['app'] = $cmd;
                 self::$applications[$cmd]['path'] = PathUtility::dirname($cmd) . '/';
@@ -244,7 +244,7 @@ class CommandUtility
             if (!$handler) {
                 return -1;
             }
-            $handler .= ' ' . $handlerOpt . ' ';
+            $handler .= ' ' . escapeshellcmd($handlerOpt) . ' ';
         }
 
         // Command