Skip to content

Commit 2c04eea

Browse files
ohaderohader
committed
[SECURITY] Enclose file type scope when invoking ImageMagick
In order to enclose and avoid type guessing done by ImageMagick based on mime-type and internal file content checks, new value object class ImageMagickFile has been introduced as guard for those invocations. Resolves: #87588 Releases: master, 9.5, 8.7 Security-Commit: 9db2aadc177ba453d935060fabf2d72fd80d0747 Security-Bulletin: TYPO3-CORE-SA-2019-012 Change-Id: Iea1dcd5d2a9af2a37dfaeb64fc31d7322f544e70 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60700 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
1 parent 0c94aba commit 2c04eea

File tree

18 files changed

+9547
-18
lines changed

18 files changed

+9547
-18
lines changed

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2456,8 +2456,11 @@ public function imageMagickIdentify($imagefile)
24562456
return null;
24572457
}
24582458

2459-
$frame = $this->addFrameSelection ? '[0]' : '';
2460-
$cmd = CommandUtility::imageMagickCommand('identify', CommandUtility::escapeShellArgument($imagefile) . $frame);
2459+
$frame = $this->addFrameSelection ? 0 : null;
2460+
$cmd = CommandUtility::imageMagickCommand(
2461+
'identify',
2462+
ImageMagickFile::fromFilePath($imagefile, $frame)
2463+
);
24612464
$returnVal = [];
24622465
CommandUtility::exec($cmd, $returnVal);
24632466
$splitstring = array_pop($returnVal);
@@ -2500,8 +2503,13 @@ public function imageMagickExec($input, $output, $params, $frame = 0)
25002503
}
25012504
// If addFrameSelection is set in the Install Tool, a frame number is added to
25022505
// select a specific page of the image (by default this will be the first page)
2503-
$frame = $this->addFrameSelection ? '[' . (int)$frame . ']' : '';
2504-
$cmd = CommandUtility::imageMagickCommand('convert', $params . ' ' . CommandUtility::escapeShellArgument($input . $frame) . ' ' . CommandUtility::escapeShellArgument($output));
2506+
$frame = $this->addFrameSelection ? (int)$frame : null;
2507+
$cmd = CommandUtility::imageMagickCommand(
2508+
'convert',
2509+
$params
2510+
. ' ' . ImageMagickFile::fromFilePath($input, $frame)
2511+
. ' ' . CommandUtility::escapeShellArgument($output)
2512+
);
25052513
$this->IM_commands[] = [$output, $cmd];
25062514
$ret = CommandUtility::exec($cmd);
25072515
// Change the permissions of the file
@@ -2531,9 +2539,9 @@ public function combineExec($input, $overlay, $mask, $output)
25312539
$parameters = '-compose over'
25322540
. ' -quality ' . $this->jpegQuality
25332541
. ' +matte '
2534-
. CommandUtility::escapeShellArgument($input) . ' '
2535-
. CommandUtility::escapeShellArgument($overlay) . ' '
2536-
. CommandUtility::escapeShellArgument($theMask) . ' '
2542+
. ImageMagickFile::fromFilePath($input) . ' '
2543+
. ImageMagickFile::fromFilePath($overlay) . ' '
2544+
. ImageMagickFile::fromFilePath($theMask) . ' '
25372545
. CommandUtility::escapeShellArgument($output);
25382546
$cmd = CommandUtility::imageMagickCommand('combine', $parameters);
25392547
$this->IM_commands[] = [$output, $cmd];
@@ -2578,7 +2586,7 @@ public static function gifCompress($theFile, $type)
25782586
if (@rename($theFile, $temporaryName)) {
25792587
$cmd = CommandUtility::imageMagickCommand(
25802588
'convert',
2581-
implode(' ', CommandUtility::escapeShellArguments([$temporaryName, $theFile])),
2589+
ImageMagickFile::fromFilePath($temporaryName) . ' ' . CommandUtility::escapeShellArgument($theFile),
25822590
$gfxConf['processor_path_lzw']
25832591
);
25842592
CommandUtility::exec($cmd);
@@ -2628,7 +2636,7 @@ public static function readPngGif($theFile, $output_png = false)
26282636
$newFile = Environment::getPublicPath() . '/typo3temp/assets/images/' . md5($theFile . '|' . filemtime($theFile)) . ($output_png ? '.png' : '.gif');
26292637
$cmd = CommandUtility::imageMagickCommand(
26302638
'convert',
2631-
implode(' ', CommandUtility::escapeShellArguments([$theFile, $newFile])),
2639+
ImageMagickFile::fromFilePath($theFile) . ' ' . CommandUtility::escapeShellArgument($newFile),
26322640
$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path']
26332641
);
26342642
CommandUtility::exec($cmd);

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

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
<?php
2+
declare(strict_types = 1);
3+
namespace TYPO3\CMS\Core\Imaging;
4+
5+
/*
6+
* This file is part of the TYPO3 CMS project.
7+
*
8+
* It is free software; you can redistribute it and/or modify it under
9+
* the terms of the GNU General Public License, either version 2
10+
* of the License, or any later version.
11+
*
12+
* For the full copyright and license information, please read the
13+
* LICENSE.txt file that was distributed with this source code.
14+
*
15+
* The TYPO3 project - inspiring people to share!
16+
*/
17+
18+
use TYPO3\CMS\Core\Exception;
19+
use TYPO3\CMS\Core\Type\File\FileInfo;
20+
use TYPO3\CMS\Core\Utility\CommandUtility;
21+
use TYPO3\CMS\Core\Utility\GeneralUtility;
22+
23+
/**
24+
* Value object for file to be used for ImageMagick/GraphicsMagick invocation when
25+
* being used as input file (implies and requires that file exists for some evaluations).
26+
*/
27+
class ImageMagickFile
28+
{
29+
/**
30+
* Path to input file to be processed
31+
*
32+
* @var string
33+
*/
34+
protected $filePath;
35+
36+
/**
37+
* Frame to be used (of multi-page document, e.g. PDF)
38+
*
39+
* @var int|null
40+
*/
41+
protected $frame;
42+
43+
/**
44+
* Whether file actually exists
45+
*
46+
* @var bool
47+
*/
48+
protected $fileExists;
49+
50+
/**
51+
* File extension as given in $filePath (e.g. 'file.png' -> 'png')
52+
*
53+
* @var string
54+
*/
55+
protected $fileExtension;
56+
57+
/**
58+
* Resolved mime-type of file
59+
*
60+
* @var string
61+
*/
62+
protected $mimeType;
63+
64+
/**
65+
* Resolved extension for mime-type (e.g. 'image/png' -> 'png')
66+
* (might be empty if not defined in magic.mime database)
67+
*
68+
* @var string[]
69+
* @see FileInfo::getMimeExtensions()
70+
*/
71+
protected $mimeExtensions = [];
72+
73+
/**
74+
* Result to be used for ImageMagick/GraphicsMagick invocation containing
75+
* combination of resolved format prefix, $filePath and frame escaped to be
76+
* used as CLI argument (e.g. "'png:file.png'")
77+
*
78+
* @var string
79+
*/
80+
protected $asArgument;
81+
82+
/**
83+
* File extensions that directly can be used (and are considered to be safe).
84+
*
85+
* @var string[]
86+
*/
87+
protected $allowedExtensions = ['png', 'jpg', 'jpeg', 'gif', 'webp', 'tif', 'tiff', 'bmp', 'pcx', 'tga', 'ico'];
88+
89+
/**
90+
* File extensions that never shall be used.
91+
*
92+
* @var string[]
93+
*/
94+
protected $deniedExtensions = ['epi', 'eps', 'eps2', 'eps3', 'epsf', 'epsi', 'ept', 'ept2', 'ept3', 'msl', 'ps', 'ps2', 'ps3'];
95+
96+
/**
97+
* File mime-types that have to be matching. Adding custom mime-types is possible using
98+
* $GLOBALS['TYPO3_CONF_VARS']['SYS']['FileInfo']['fileExtensionToMimeType']
99+
*
100+
* @var string[]
101+
* @see FileInfo::getMimeExtensions()
102+
*/
103+
protected $mimeTypeExtensionMap = [
104+
'image/png' => 'png',
105+
'image/jpeg' => 'jpg',
106+
'image/gif' => 'gif',
107+
'image/heic' => 'heic',
108+
'image/heif' => 'heif',
109+
'image/webp' => 'webp',
110+
'image/svg' => 'svg',
111+
'image/svg+xml' => 'svg',
112+
'image/tiff' => 'tif',
113+
'application/pdf' => 'pdf',
114+
];
115+
116+
/**
117+
* @param string $filePath
118+
* @param int|null $frame
119+
* @return ImageMagickFile
120+
*/
121+
public static function fromFilePath(string $filePath, int $frame = null): self
122+
{
123+
return GeneralUtility::makeInstance(
124+
static::class,
125+
$filePath,
126+
$frame
127+
);
128+
}
129+
130+
/**
131+
* @param string $filePath
132+
* @param int|null $frame
133+
* @throws Exception
134+
*/
135+
public function __construct(string $filePath, int $frame = null)
136+
{
137+
$this->frame = $frame;
138+
$this->fileExists = file_exists($filePath);
139+
$this->filePath = $filePath;
140+
$this->fileExtension = pathinfo($filePath, PATHINFO_EXTENSION);
141+
142+
if ($this->fileExists) {
143+
$fileInfo = $this->getFileInfo($filePath);
144+
$this->mimeType = $fileInfo->getMimeType();
145+
$this->mimeExtensions = $fileInfo->getMimeExtensions();
146+
}
147+
148+
$this->asArgument = $this->escape(
149+
$this->resolvePrefix() . $this->filePath
150+
. ($this->frame !== null ? '[' . $this->frame . ']' : '')
151+
);
152+
}
153+
154+
/**
155+
* @return string
156+
*/
157+
public function __toString(): string
158+
{
159+
return $this->asArgument;
160+
}
161+
162+
/**
163+
* Resolves according ImageMagic/GraphicsMagic format (e.g. 'png:', 'jpg:', ...).
164+
* + in case mime-type could be resolved and is configured, it takes precedence
165+
* + otherwise resolved mime-type extension of mime.magick database is used if available
166+
* (includes custom settings with $GLOBALS['TYPO3_CONF_VARS']['SYS']['FileInfo']['fileExtensionToMimeType'])
167+
* + otherwise "safe" and allowed file extension is used (jpg, png, gif, webp, tif, ...)
168+
* + potentially malicious script formats (eps, ps, ...) are not allowed
169+
*
170+
* @return string
171+
* @throws Exception
172+
*/
173+
protected function resolvePrefix(): string
174+
{
175+
$prefixExtension = null;
176+
$fileExtension = strtolower($this->fileExtension);
177+
if ($this->mimeType !== null && !empty($this->mimeTypeExtensionMap[$this->mimeType])) {
178+
$prefixExtension = $this->mimeTypeExtensionMap[$this->mimeType];
179+
} elseif (!empty($this->mimeExtensions) && strpos((string)$this->mimeType, 'image/') === 0) {
180+
$prefixExtension = $this->mimeExtensions[0];
181+
} elseif ($this->isInAllowedExtensions($fileExtension)) {
182+
$prefixExtension = $fileExtension;
183+
}
184+
if ($prefixExtension !== null && !in_array(strtolower($prefixExtension), $this->deniedExtensions, true)) {
185+
return $prefixExtension . ':';
186+
}
187+
throw new Exception(
188+
sprintf(
189+
'Unsupported file %s (%s)',
190+
basename($this->filePath),
191+
$this->mimeType ?? 'unknown'
192+
),
193+
1550060977
194+
);
195+
}
196+
197+
/**
198+
* @param string $value
199+
* @return string
200+
*/
201+
protected function escape(string $value): string
202+
{
203+
return CommandUtility::escapeShellArgument($value);
204+
}
205+
206+
/**
207+
* @param string $extension
208+
* @return bool
209+
*/
210+
protected function isInAllowedExtensions(string $extension): bool
211+
{
212+
return in_array($extension, $this->allowedExtensions, true);
213+
}
214+
215+
/**
216+
* @param string $filePath
217+
* @return FileInfo
218+
*/
219+
protected function getFileInfo(string $filePath): FileInfo
220+
{
221+
return GeneralUtility::makeInstance(FileInfo::class, $filePath);
222+
}
223+
}

‎typo3/sysext/core/Classes/Resource/OnlineMedia/Processing/PreviewProcessing.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
use TYPO3\CMS\Core\Core\Environment;
1818
use TYPO3\CMS\Core\Imaging\GraphicalFunctions;
19+
use TYPO3\CMS\Core\Imaging\ImageMagickFile;
1920
use TYPO3\CMS\Core\Resource\Driver\DriverInterface;
2021
use TYPO3\CMS\Core\Resource\File;
2122
use TYPO3\CMS\Core\Resource\OnlineMedia\Helpers\OnlineMediaHelperRegistry;
@@ -138,11 +139,10 @@ protected function resizeImage($originalFileName, $temporaryFileName, $configura
138139
$arguments = CommandUtility::escapeShellArguments([
139140
'width' => $configuration['width'],
140141
'height' => $configuration['height'],
141-
'originalFileName' => $originalFileName,
142-
'temporaryFileName' => $temporaryFileName,
143142
]);
144-
$parameters = '-sample ' . $arguments['width'] . 'x' . $arguments['height'] . ' '
145-
. $arguments['originalFileName'] . '[0] ' . $arguments['temporaryFileName'];
143+
$parameters = '-sample ' . $arguments['width'] . 'x' . $arguments['height']
144+
. ' ' . ImageMagickFile::fromFilePath($originalFileName, 0)
145+
. ' ' . CommandUtility::escapeShellArgument($temporaryFileName);
146146

147147
$cmd = CommandUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
148148
CommandUtility::exec($cmd);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
use TYPO3\CMS\Core\Imaging\GraphicalFunctions;
18+
use TYPO3\CMS\Core\Imaging\ImageMagickFile;
1819
use TYPO3\CMS\Core\Resource\File;
1920
use TYPO3\CMS\Core\Utility\CommandUtility;
2021
use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -151,11 +152,10 @@ protected function generatePreviewFromFile(File $file, array $configuration, $ta
151152
$arguments = CommandUtility::escapeShellArguments([
152153
'width' => $configuration['width'],
153154
'height' => $configuration['height'],
154-
'originalFileName' => $originalFileName,
155-
'targetFilePath' => $targetFilePath,
156155
]);
157-
$parameters = '-sample ' . $arguments['width'] . 'x' . $arguments['height'] . ' '
158-
. $arguments['originalFileName'] . '[0] ' . $arguments['targetFilePath'];
156+
$parameters = '-sample ' . $arguments['width'] . 'x' . $arguments['height']
157+
. ' ' . ImageMagickFile::fromFilePath($originalFileName, 0)
158+
. ' ' . CommandUtility::escapeShellArgument($targetFilePath);
159159

160160
$cmd = CommandUtility::imageMagickCommand('convert', $parameters) . ' 2>&1';
161161
CommandUtility::exec($cmd);

0 commit comments

Comments
 (0)