Skip to content
This repository was archived by the owner on May 25, 2023. It is now read-only.

Commit fe44d34

Browse files
committed
SECURITY: Verify file signatures before image processing.
This mitigates potential vulnerabilities in ImageMagick when handling input files other than GIF/JPEG/PNG. However this does not prevent all potential vulnerabilities with ImageMagick. It is therefore recommended to disable all non-required ImageMagick coders via policy.xml. See also: https://imagetragick.com/ https://www.kb.cert.org/vuls/id/332928 https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=imagemagick
1 parent 56ebf0f commit fe44d34

File tree

1 file changed

+31
-20
lines changed

1 file changed

+31
-20
lines changed

server/php/UploadHandler.php

+31-20
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ class UploadHandler
3838
'image_resize' => 'Failed to resize image'
3939
);
4040

41+
protected const IMAGETYPE_GIF = 1;
42+
protected const IMAGETYPE_JPEG = 2;
43+
protected const IMAGETYPE_PNG = 3;
44+
4145
protected $image_objects = array();
4246

4347
public function __construct($options = null, $initialize = true, $error_messages = null) {
@@ -114,9 +118,7 @@ public function __construct($options = null, $initialize = true, $error_messages
114118
'min_file_size' => 1,
115119
// The maximum number of files for the upload directory:
116120
'max_number_of_files' => null,
117-
// Defines which files are handled as image files:
118-
'image_file_types' => '/\.(gif|jpe?g|png)$/i',
119-
// Use exif_imagetype on all files to correct file extensions:
121+
// Reads first file bytes to identify and correct file extensions:
120122
'correct_image_extensions' => false,
121123
// Image resolution restrictions:
122124
'max_width' => null,
@@ -163,7 +165,7 @@ public function __construct($options = null, $initialize = true, $error_messages
163165
'max_width' => 800,
164166
'max_height' => 600
165167
),
166-
*/
168+
*/
167169
'thumbnail' => array(
168170
// Uncomment the following to use a defined directory for the thumbnails
169171
// instead of a subdirectory based on the version identifier.
@@ -433,9 +435,8 @@ protected function validate($uploaded_file, $file, $error, $index) {
433435
$min_width = @$this->options['min_width'];
434436
$min_height = @$this->options['min_height'];
435437
if (($max_width || $max_height || $min_width || $min_height)
436-
&& preg_match($this->options['image_file_types'], $file->name)) {
438+
&& $this->is_valid_image_file($uploaded_file)) {
437439
list($img_width, $img_height) = $this->get_image_size($uploaded_file);
438-
439440
// If we are auto rotating the image by default, do the checks on
440441
// the correct orientation
441442
if (
@@ -449,7 +450,6 @@ function_exists('exif_read_data') &&
449450
$img_height = $tmp;
450451
unset($tmp);
451452
}
452-
453453
}
454454
if (!empty($img_width)) {
455455
if ($max_width && $img_width > $max_width) {
@@ -511,16 +511,15 @@ protected function fix_file_extension($file_path, $name, $size, $type, $error,
511511
preg_match('/^image\/(gif|jpe?g|png)/', $type, $matches)) {
512512
$name .= '.'.$matches[1];
513513
}
514-
if ($this->options['correct_image_extensions'] &&
515-
function_exists('exif_imagetype')) {
516-
switch (@exif_imagetype($file_path)){
517-
case IMAGETYPE_JPEG:
514+
if ($this->options['correct_image_extensions']) {
515+
switch ($this->imagetype($file_path)) {
516+
case self::IMAGETYPE_JPEG:
518517
$extensions = array('jpg', 'jpeg');
519518
break;
520-
case IMAGETYPE_PNG:
519+
case self::IMAGETYPE_PNG:
521520
$extensions = array('png');
522521
break;
523-
case IMAGETYPE_GIF:
522+
case self::IMAGETYPE_GIF:
524523
$extensions = array('gif');
525524
break;
526525
}
@@ -1063,15 +1062,27 @@ protected function destroy_image_object($file_path) {
10631062
}
10641063
}
10651064

1066-
protected function is_valid_image_file($file_path) {
1067-
if (!preg_match($this->options['image_file_types'], $file_path)) {
1068-
return false;
1065+
protected function imagetype($file_path) {
1066+
$fp = fopen($file_path, 'r');
1067+
$data = fread($fp, 4);
1068+
fclose($fp);
1069+
// GIF: 47 49 46
1070+
if (substr($data, 0, 3) === 'GIF') {
1071+
return self::IMAGETYPE_GIF;
10691072
}
1070-
if (function_exists('exif_imagetype')) {
1071-
return @exif_imagetype($file_path);
1073+
// JPG: FF D8
1074+
if (bin2hex(substr($data, 0, 2)) === 'ffd8') {
1075+
return self::IMAGETYPE_JPEG;
10721076
}
1073-
$image_info = $this->get_image_size($file_path);
1074-
return $image_info && $image_info[0] && $image_info[1];
1077+
// PNG: 89 50 4E 47
1078+
if (bin2hex(@$data[0]).substr($data, 1, 4) === '89PNG') {
1079+
return self::IMAGETYPE_PNG;
1080+
}
1081+
return false;
1082+
}
1083+
1084+
protected function is_valid_image_file($file_path) {
1085+
return !!$this->imagetype($file_path);
10751086
}
10761087

10771088
protected function handle_image_file($file_path, $file) {

0 commit comments

Comments
 (0)