From 150ed8fb6f516664eca579568548a6366a319905 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Thu, 25 Oct 2018 18:00:08 +0100 Subject: [PATCH 1/8] Check mime type when uploading files --- admin-dev/filemanager/config/config.php | 8 ++++++++ admin-dev/filemanager/upload.php | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/admin-dev/filemanager/config/config.php b/admin-dev/filemanager/config/config.php index 46677c9487a95..334e16a92565d 100644 --- a/admin-dev/filemanager/config/config.php +++ b/admin-dev/filemanager/config/config.php @@ -111,6 +111,14 @@ $ext=array_merge($ext_img, $ext_file, $ext_misc, $ext_video, $ext_music); //allowed extensions +//********************** +//Allowed mime types +//********************** +$mime_img = array('image/jpeg', 'image/png', 'image/gif', 'image/bmp', 'image/tiff', 'image/svg'); +$mime_file = array('application/pdf'); +$mime_video = array('video/mpeg', 'video/mp4', 'video/x-msvideo', 'audio/x-ms-wma', 'video/x-flv', 'video/webm'); + +$mime = array_merge($mime_img, $mime_file, $mime_video); /****************** * AVIARY config diff --git a/admin-dev/filemanager/upload.php b/admin-dev/filemanager/upload.php index 488d51a4d39a6..0f2dfb5f458d6 100644 --- a/admin-dev/filemanager/upload.php +++ b/admin-dev/filemanager/upload.php @@ -38,7 +38,10 @@ if (!empty($_FILES) && isset($_FILES['file']) && $_FILES['file']['size']) { $info = pathinfo($_FILES['file']['name']); - if (isset($info['extension']) && in_array(fix_strtolower($info['extension']), $ext)) { + if (isset($info['extension']) + && in_array(fix_strtolower($info['extension']), $ext) + && in_array(mime_content_type($_FILES['file']['tmp_name']), $mime) + ) { $tempFile = $_FILES['file']['tmp_name']; $targetPath = $storeFolder; From 4c6958f40cf7faa58207a203f3a5523cc8015148 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Fri, 26 Oct 2018 17:38:33 +0100 Subject: [PATCH 2/8] Remove filemanager action image_size --- admin-dev/filemanager/ajax_calls.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/admin-dev/filemanager/ajax_calls.php b/admin-dev/filemanager/ajax_calls.php index 47c5918247f1f..3ae2ddee75f8d 100644 --- a/admin-dev/filemanager/ajax_calls.php +++ b/admin-dev/filemanager/ajax_calls.php @@ -24,17 +24,6 @@ if (isset($_GET['descending'])) { $_SESSION['descending'] = $_GET['descending'] === 'true'; } - break; - case 'image_size': - if (realpath(dirname(_PS_ROOT_DIR_.$_POST['path'])) != realpath(_PS_ROOT_DIR_.$upload_dir)) { - die(); - } - $pos = strpos($_POST['path'], $upload_dir); - if ($pos !== false) { - $info = getimagesize(substr_replace($_POST['path'], $current_path, $pos, strlen($upload_dir))); - echo json_encode($info); - } - break; case 'save_img': $info = pathinfo($_POST['name']); From 5aeb936baea3728040c1282b04605e5cd01a84ee Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Fri, 26 Oct 2018 13:24:47 +0200 Subject: [PATCH 3/8] Avoid current folder from being deleted --- admin-dev/filemanager/execute.php | 11 ++++++++--- admin-dev/filemanager/include/utils.php | 13 +++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/admin-dev/filemanager/execute.php b/admin-dev/filemanager/execute.php index 5b8a3ff86ddc1..8661490e5a09f 100644 --- a/admin-dev/filemanager/execute.php +++ b/admin-dev/filemanager/execute.php @@ -10,10 +10,10 @@ die('wrong path'); } -$thumb_pos = strpos($_POST['path_thumb'], $thumbs_base_path); -if ($thumb_pos === false - || preg_match('/\.{1,2}[\/|\\\]/', $_POST['path_thumb']) !== 0 +if (preg_match('/\.{1,2}[\/|\\\]/', $_POST['path_thumb']) !== 0 || preg_match('/\.{1,2}[\/|\\\]/', $_POST['path']) !== 0 + || strpos(realpath($_POST['path']), realpath($current_path)) !== 0 + || strpos(realpath($_POST['path_thumb']), realpath($thumbs_base_path)) !== 0 ) { die('wrong path'); } @@ -70,6 +70,7 @@ switch ($_GET['action']) { case 'delete_file': if ($delete_files) { + stopIfSameDir($current_path, array($path, $path_thumb)); unlink($path); if (file_exists($path_thumb)) { unlink($path_thumb); @@ -102,9 +103,11 @@ break; case 'delete_folder': if ($delete_folders) { + stopIfSameDir($current_path, array($path, $path_thumb)); if (is_dir($path_thumb)) { deleteDir($path_thumb); } + if (is_dir($path)) { deleteDir($path); if ($fixed_image_creation) { @@ -132,6 +135,7 @@ $name = str_replace('.', '', $name); if (!empty($name)) { + stopIfSameDir($current_path, array($path, $path_thumb)); if (!rename_folder($path, $name, $transliteration)) { die(lang_Rename_existing_folder); } @@ -154,6 +158,7 @@ if ($rename_files) { $name = fix_filename($name, $transliteration); if (!empty($name)) { + stopIfSameDir($current_path, array($path, $path_thumb)); if (!rename_file($path, $name, $transliteration)) { die(lang_Rename_existing_file); } diff --git a/admin-dev/filemanager/include/utils.php b/admin-dev/filemanager/include/utils.php index e3188afa9f4ef..caf5d22f6c8f5 100644 --- a/admin-dev/filemanager/include/utils.php +++ b/admin-dev/filemanager/include/utils.php @@ -358,3 +358,16 @@ function get_file_by_url($url) return $data; } + +/** + * @param string $sourcePath + * @param array $paths List of paths to compare + */ +function stopIfSameDir($sourcePath, array $paths) +{ + foreach ($paths as $path) { + if (realpath($sourcePath) === realpath($path)) { + die('wrong_path'); + } + } +} From c70a40d575b1092cd4870bd4386227a279693341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Fri, 26 Oct 2018 10:35:35 +0200 Subject: [PATCH 4/8] [Security] fixed arbitrary image write/overwrite in Windows installation --- admin-dev/filemanager/ajax_calls.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/admin-dev/filemanager/ajax_calls.php b/admin-dev/filemanager/ajax_calls.php index 3ae2ddee75f8d..9802188247f0b 100644 --- a/admin-dev/filemanager/ajax_calls.php +++ b/admin-dev/filemanager/ajax_calls.php @@ -27,15 +27,18 @@ break; case 'save_img': $info = pathinfo($_POST['name']); - if (strpos($_POST['path'], '/') === 0 - || strpos($_POST['path'], '../') !== false - || strpos($_POST['path'], './') === 0 - || strpos($_POST['url'], 'http://featherfiles.aviary.com/') !== 0 - || $_POST['name'] != fix_filename($_POST['name'], $transliteration) + $filename = $_POST['name']; + $path_pos = $_POST['path']; + + if (preg_match('/\.{1,2}[\/|\\\]/', $path_pos) !== 0 + || $filename !== fix_filename($filename, $transliteration) || !in_array(strtolower($info['extension']), array('jpg', 'jpeg', 'png')) + || strpos($_POST['url'], 'http://featherfiles.aviary.com/') !== 0 + ) { die('wrong data'); } + $image_data = get_file_by_url($_POST['url']); if ($image_data === false) { die('file could not be loaded'); From f75dd420d69ad792602467789fe6049858c31aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 30 Oct 2018 14:43:27 +0100 Subject: [PATCH 5/8] Added extra check on file extension and mime type --- admin-dev/filemanager/ajax_calls.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/admin-dev/filemanager/ajax_calls.php b/admin-dev/filemanager/ajax_calls.php index 9802188247f0b..ff7ba198a9268 100644 --- a/admin-dev/filemanager/ajax_calls.php +++ b/admin-dev/filemanager/ajax_calls.php @@ -27,6 +27,7 @@ break; case 'save_img': $info = pathinfo($_POST['name']); + $filename = $_POST['name']; $path_pos = $_POST['path']; @@ -34,6 +35,8 @@ || $filename !== fix_filename($filename, $transliteration) || !in_array(strtolower($info['extension']), array('jpg', 'jpeg', 'png')) || strpos($_POST['url'], 'http://featherfiles.aviary.com/') !== 0 + || !isset($info['extension']) + || !in_array(mime_content_type($filename), $mime_img) ) { die('wrong data'); From 3871154796f63e2478d19cb039ad6e0ab8b0cd17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Andrieu?= Date: Tue, 30 Oct 2018 15:14:54 +0100 Subject: [PATCH 6/8] Address @farisv comments --- admin-dev/filemanager/ajax_calls.php | 11 ++++++++++- composer.json | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/admin-dev/filemanager/ajax_calls.php b/admin-dev/filemanager/ajax_calls.php index ff7ba198a9268..24c0e6eed9880 100644 --- a/admin-dev/filemanager/ajax_calls.php +++ b/admin-dev/filemanager/ajax_calls.php @@ -36,13 +36,22 @@ || !in_array(strtolower($info['extension']), array('jpg', 'jpeg', 'png')) || strpos($_POST['url'], 'http://featherfiles.aviary.com/') !== 0 || !isset($info['extension']) - || !in_array(mime_content_type($filename), $mime_img) ) { die('wrong data'); } $image_data = get_file_by_url($_POST['url']); + + $tmp = tempnam(sys_get_temp_dir(), 'img'); + file_put_contents($tmp, $image_data); + $mime = mime_content_type($tmp); + unlink($tmp); + + if (!in_array($mime, $mime_img)) { + die('wrong data'); + } + if ($image_data === false) { die('file could not be loaded'); } diff --git a/composer.json b/composer.json index dbd71c3c347f0..eb0ccf16ece84 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,7 @@ "ext-curl": "*", "ext-intl": "*", "ext-zip": "*", + "ext-fileinfo": "*", "beberlei/DoctrineExtensions": "^1.0", "composer/ca-bundle": "^1.0", "composer/installers": "^1.0.21", From d65c608c54cd7b4322bda21d8a7352e9f71c43bd Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Tue, 30 Oct 2018 17:30:41 +0100 Subject: [PATCH 7/8] fix path checking in execute --- admin-dev/filemanager/execute.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin-dev/filemanager/execute.php b/admin-dev/filemanager/execute.php index 8661490e5a09f..397849d8d71a1 100644 --- a/admin-dev/filemanager/execute.php +++ b/admin-dev/filemanager/execute.php @@ -12,7 +12,7 @@ if (preg_match('/\.{1,2}[\/|\\\]/', $_POST['path_thumb']) !== 0 || preg_match('/\.{1,2}[\/|\\\]/', $_POST['path']) !== 0 - || strpos(realpath($_POST['path']), realpath($current_path)) !== 0 + || strpos(realpath($current_path.$_POST['path']), realpath($current_path)) !== 0 || strpos(realpath($_POST['path_thumb']), realpath($thumbs_base_path)) !== 0 ) { die('wrong path'); From 7b416e265ce3ead3b1be5a113cd7a24648c2683d Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Wed, 31 Oct 2018 12:01:54 +0000 Subject: [PATCH 8/8] Check realpath only when folder/file actually exists --- admin-dev/filemanager/execute.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/admin-dev/filemanager/execute.php b/admin-dev/filemanager/execute.php index 397849d8d71a1..e823838743e07 100644 --- a/admin-dev/filemanager/execute.php +++ b/admin-dev/filemanager/execute.php @@ -10,10 +10,13 @@ die('wrong path'); } +$realPath = realpath($current_path.$_POST['path']); +$realPathThumb = realpath($_POST['path_thumb']); + if (preg_match('/\.{1,2}[\/|\\\]/', $_POST['path_thumb']) !== 0 || preg_match('/\.{1,2}[\/|\\\]/', $_POST['path']) !== 0 - || strpos(realpath($current_path.$_POST['path']), realpath($current_path)) !== 0 - || strpos(realpath($_POST['path_thumb']), realpath($thumbs_base_path)) !== 0 + || ($realPath && strpos($realPath, realpath($current_path)) !== 0) + || ($realPathThumb && strpos($realPathThumb, realpath($thumbs_base_path)) !== 0) ) { die('wrong path'); }