From 85a6ed25c4b5e3ddae685941359169bd1c0b8a65 Mon Sep 17 00:00:00 2001 From: "Woojae, Park" Date: Thu, 19 Mar 2020 15:08:25 +0700 Subject: [PATCH 1/4] MC-29272: 896b5aceef7 MC-29272 - FIX: Can't validate uploaded file for tmp folder mismatching #25835 when upload file, php returns destination path of symlinked tmp folder. but sys_get_temp_dir() function only return target path of symlinked folder. this mismatch made it can't pass the validation. this commit validate on both target path and destination path if symlink found. I couldn't find php $_FILE source reference, so I put both pathes on validation. --- lib/internal/Magento/Framework/File/Uploader.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/File/Uploader.php b/lib/internal/Magento/Framework/File/Uploader.php index f9b41709ec7c8..34954c8024ad1 100644 --- a/lib/internal/Magento/Framework/File/Uploader.php +++ b/lib/internal/Magento/Framework/File/Uploader.php @@ -612,8 +612,16 @@ private function validateFileId(array $fileId): void $tmpName = trim($fileId['tmp_name']); if (preg_match('/\.\.(\\\|\/)/', $tmpName) !== 1) { + $sysTmpFolder = sys_get_temp_dir(); + $sysTmpFolderTarget = ''; + /* check for symlinks */ + if (is_link($sysTmpFolder)) { + $sysTmpFolderTarget = readlink($link); + } + $allowedFolders = [ - sys_get_temp_dir(), + $sysTmpFolder, + $sysTmpFolderTarget, $this->directoryList->getPath(DirectoryList::MEDIA), $this->directoryList->getPath(DirectoryList::VAR_DIR), $this->directoryList->getPath(DirectoryList::TMP), From 18ca553fd11e08349c49e61b72e8fa160bad8d5d Mon Sep 17 00:00:00 2001 From: "Woojae, Park" Date: Fri, 3 Apr 2020 21:53:27 +0700 Subject: [PATCH 2/4] FIX: symlink check failure on macOS - thanks to @hostep --- lib/internal/Magento/Framework/File/Uploader.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/internal/Magento/Framework/File/Uploader.php b/lib/internal/Magento/Framework/File/Uploader.php index 34954c8024ad1..855ab8688ea2d 100644 --- a/lib/internal/Magento/Framework/File/Uploader.php +++ b/lib/internal/Magento/Framework/File/Uploader.php @@ -613,15 +613,11 @@ private function validateFileId(array $fileId): void if (preg_match('/\.\.(\\\|\/)/', $tmpName) !== 1) { $sysTmpFolder = sys_get_temp_dir(); - $sysTmpFolderTarget = ''; - /* check for symlinks */ - if (is_link($sysTmpFolder)) { - $sysTmpFolderTarget = readlink($link); - } - + $sysTmpFolderRealPath = realpath($sysTmpFolder); + $allowedFolders = [ $sysTmpFolder, - $sysTmpFolderTarget, + $sysTmpFolderRealPath, $this->directoryList->getPath(DirectoryList::MEDIA), $this->directoryList->getPath(DirectoryList::VAR_DIR), $this->directoryList->getPath(DirectoryList::TMP), From 6297bb4a69c8289758f640301bf0ad9429fcdc59 Mon Sep 17 00:00:00 2001 From: baranada Date: Mon, 28 Sep 2020 11:01:38 +0700 Subject: [PATCH 3/4] ADD: test code for validating uploaded file --- .../Framework/File/Test/Unit/UploaderTest.php | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php b/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php index f935b2f37e407..b69645e17a68c 100644 --- a/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php +++ b/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php @@ -15,6 +15,23 @@ */ class UploaderTest extends TestCase { + /** + * Uploader model + * + * @var \Magento\Framework\File\Uploader + */ + protected $_model; + + protected function setUp(): void + { + + } + + protected function tearDown(): void + { + + } + /** * @param string $fileName * @param string|bool $expectedCorrectedFileName @@ -67,4 +84,71 @@ public function getCorrectFileNameProvider() ] ]; } + + /** + * + */ + public function testFileSingleUpload() + { + $sysTmpFolder = sys_get_temp_dir(); + $fileId = "testimagepngfile"; + $uploaded_filename = $sysTmpFolder . "/" . $fileId; + $imagestring = "data:image/webp;base64,UklGRoYEAABXRUJQVlA4IHoEAACQKgCdASoAAQABPkkkjUUioiETPpSQKASEsrdwtyDMUgcmO4Pxb8o9zG63+Of5R5L/YfzA+QP6/9vHzs/zP+A9of/A9QD9KOln5gP13/ZD3hv9V1AH9G6kL+n+oB+wHlAfD5+437Ae0BqwPV0DE1vv5JOEs6L1cvSRuOfknd3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSgj7idUK6lLlFRs5REdk5bl1Cj4V10Dc6/VbAeI5hP2Mz7Ad8NflHEg1OcMNThZsibQMuI5i+BBgcNgDZAGRilVaV97A63fEnhUsHAh+34kOZ7o4X/NqLPuk87UBQeB22PMUG5SyELop/1fp/OOReuY8ErnnlvVt5sy2FbAeIwhYjFZokINgg1aQVjlogGfSINxAUU6ZAyHOcWKUnm561l1ixXd3d3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSWAAP13AdVi8Xqe4VLs+URR2fUf81LdBdC0wcP/sRaAAKcF6uXyM88AoR/IygHdNUBL/8aQC7jOX6aXAOyBc1K2VQ+CPyDp09nbHKYJkoS95ltNPTkGdZFlnr+0URtVLKWlmoBjRoJBnLkxgPBan+q/7v0sFaAgqUTPVARxi0KeBvyE2fYvtN0wXfsYpll2ZyTFaWv74g+NvwOIorfxppmtw1Aoe44JrkNfy92+kxmHl09JB5IWndyUZRPg0ENUORNjjrfwR53QUrAv0Pbw3+KzCsmRClX/6j5qHi+MfbVkmbixgheNw1TRP+3vwHXXuzEZM9TBO9XANE2AzPm878LDSXDOwItzSeQesOJ62rm1UQvWaVDgH2Kn7M+8xwg+T/+OjC+iY095CFjS9okvc+qsvdTPVwAlO7LtBI0EIPzRzF5Gp/c/CFXvKUexvfh0wdgpZaH/mhYsOOYQ1+soKHvHXb8NNEASvkpeRQQ4cgkQeZ/n+265EAlN9vhsykU8uXhZMStw7o1Idw9eCuibOJ1q9fsUHxPsvdcdyKs6wWk+nD/AQNd6QoTqJzEJdsktuk7OmIPUMoHaVM8kQapQgGTe7ZzbSoFUgTNBpbI/v54p/QDvwLZtdANku5UFSccarNnogJ7N+vS0JAcldFM4Xktoll84uqcC+2xZIBAGyrliTzRiLwm2CMjXgif2ln3Kjoyhg7+VIHP5u6jkHupENMNqzDQCwxlAIZ/mZp6PTaakjeTLsEPPPQlP+HapL566KF1CLCjgDgyWkOPLlDPmi8UrpWJM4p9ujO2RAd5UT9KuIOrTRPTaFnL/Z3OmIwwnw9UKaRMXJyeGQL01KGd2+xR8WYSiFsv70JlpBI3+srAMzPIUll3LDCN7HfKyg9b34wav8xZWi8KcRgfRmRUtZ1y9JCMD+sTeve2bR0WxgWADnPLeEHRzPVh2b/invd69EQjklxr6vYcUllQuKf2NXXbpTXnPgBlUX+mCJFcAhtrgHCTQggsxeQT14ylsCgRtMvJHskl979QMxjLIwAF44RAQMYccBwblWgDiDoAAAAA="; + $data = file_put_contents($uploaded_filename, base64_decode($imagestring)); + + $_FILES = + array($fileId=>array( + "name"=>"testimage.png", + "type"=> "image/png", + "tmp_name"=>$uploaded_filename, + "error"=>0, + "size"=>1166 + )); + + $this->_model = new Uploader($fileId); + + $this->assertEquals( + file_exists($uploaded_filename), + true); + + unset($this->_model); + } + + /** + * + */ + public function testFileMultipleUpload() + { + $sysTmpFolder = sys_get_temp_dir(); + $fileId = "testimagepngfile"; + $uploaded_filename = $sysTmpFolder . "/" . $fileId; + $imagestring = "data:image/webp;base64,UklGRoYEAABXRUJQVlA4IHoEAACQKgCdASoAAQABPkkkjUUioiETPpSQKASEsrdwtyDMUgcmO4Pxb8o9zG63+Of5R5L/YfzA+QP6/9vHzs/zP+A9of/A9QD9KOln5gP13/ZD3hv9V1AH9G6kL+n+oB+wHlAfD5+437Ae0BqwPV0DE1vv5JOEs6L1cvSRuOfknd3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSgj7idUK6lLlFRs5REdk5bl1Cj4V10Dc6/VbAeI5hP2Mz7Ad8NflHEg1OcMNThZsibQMuI5i+BBgcNgDZAGRilVaV97A63fEnhUsHAh+34kOZ7o4X/NqLPuk87UBQeB22PMUG5SyELop/1fp/OOReuY8ErnnlvVt5sy2FbAeIwhYjFZokINgg1aQVjlogGfSINxAUU6ZAyHOcWKUnm561l1ixXd3d3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSWAAP13AdVi8Xqe4VLs+URR2fUf81LdBdC0wcP/sRaAAKcF6uXyM88AoR/IygHdNUBL/8aQC7jOX6aXAOyBc1K2VQ+CPyDp09nbHKYJkoS95ltNPTkGdZFlnr+0URtVLKWlmoBjRoJBnLkxgPBan+q/7v0sFaAgqUTPVARxi0KeBvyE2fYvtN0wXfsYpll2ZyTFaWv74g+NvwOIorfxppmtw1Aoe44JrkNfy92+kxmHl09JB5IWndyUZRPg0ENUORNjjrfwR53QUrAv0Pbw3+KzCsmRClX/6j5qHi+MfbVkmbixgheNw1TRP+3vwHXXuzEZM9TBO9XANE2AzPm878LDSXDOwItzSeQesOJ62rm1UQvWaVDgH2Kn7M+8xwg+T/+OjC+iY095CFjS9okvc+qsvdTPVwAlO7LtBI0EIPzRzF5Gp/c/CFXvKUexvfh0wdgpZaH/mhYsOOYQ1+soKHvHXb8NNEASvkpeRQQ4cgkQeZ/n+265EAlN9vhsykU8uXhZMStw7o1Idw9eCuibOJ1q9fsUHxPsvdcdyKs6wWk+nD/AQNd6QoTqJzEJdsktuk7OmIPUMoHaVM8kQapQgGTe7ZzbSoFUgTNBpbI/v54p/QDvwLZtdANku5UFSccarNnogJ7N+vS0JAcldFM4Xktoll84uqcC+2xZIBAGyrliTzRiLwm2CMjXgif2ln3Kjoyhg7+VIHP5u6jkHupENMNqzDQCwxlAIZ/mZp6PTaakjeTLsEPPPQlP+HapL566KF1CLCjgDgyWkOPLlDPmi8UrpWJM4p9ujO2RAd5UT9KuIOrTRPTaFnL/Z3OmIwwnw9UKaRMXJyeGQL01KGd2+xR8WYSiFsv70JlpBI3+srAMzPIUll3LDCN7HfKyg9b34wav8xZWi8KcRgfRmRUtZ1y9JCMD+sTeve2bR0WxgWADnPLeEHRzPVh2b/invd69EQjklxr6vYcUllQuKf2NXXbpTXnPgBlUX+mCJFcAhtrgHCTQggsxeQT14ylsCgRtMvJHskl979QMxjLIwAF44RAQMYccBwblWgDiDoAAAAA="; + $data = file_put_contents($uploaded_filename, base64_decode($imagestring)); + + $_FILES = + array($fileId=>array( + "name"=>array("testimage.png",""), + "type"=>array("image/png",""), + "tmp_name"=>array($uploaded_filename,""), + "error"=>array(0,4), + "size"=>array(1166,0) + )); + + $fileId=array( + "name"=>"testimage.png", + "type"=> "image/png", + "tmp_name"=>$uploaded_filename, + "error"=>0, + "size"=>1166 + ); + + $this->_model = new Uploader($fileId); + + $this->assertEquals( + file_exists($uploaded_filename), + true); + + unset($this->_model); + } + } From afcb8cbbae081b0760964b34104d0117d197f7d5 Mon Sep 17 00:00:00 2001 From: baranada Date: Fri, 9 Oct 2020 15:55:25 +0700 Subject: [PATCH 4/4] FIX: validation failure on upload. fix for unittest. --- .../Framework/File/Test/Unit/UploaderTest.php | 54 ++++++++----------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php b/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php index 7b39bba642429..ffbc51782f903 100644 --- a/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php +++ b/lib/internal/Magento/Framework/File/Test/Unit/UploaderTest.php @@ -22,16 +22,6 @@ class UploaderTest extends TestCase */ protected $_model; - protected function setUp(): void - { - - } - - protected function tearDown(): void - { - - } - /** * @param string $fileName * @param string|bool $expectedCorrectedFileName @@ -96,22 +86,21 @@ public function testFileSingleUpload() $imagestring = "data:image/webp;base64,UklGRoYEAABXRUJQVlA4IHoEAACQKgCdASoAAQABPkkkjUUioiETPpSQKASEsrdwtyDMUgcmO4Pxb8o9zG63+Of5R5L/YfzA+QP6/9vHzs/zP+A9of/A9QD9KOln5gP13/ZD3hv9V1AH9G6kL+n+oB+wHlAfD5+437Ae0BqwPV0DE1vv5JOEs6L1cvSRuOfknd3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSgj7idUK6lLlFRs5REdk5bl1Cj4V10Dc6/VbAeI5hP2Mz7Ad8NflHEg1OcMNThZsibQMuI5i+BBgcNgDZAGRilVaV97A63fEnhUsHAh+34kOZ7o4X/NqLPuk87UBQeB22PMUG5SyELop/1fp/OOReuY8ErnnlvVt5sy2FbAeIwhYjFZokINgg1aQVjlogGfSINxAUU6ZAyHOcWKUnm561l1ixXd3d3d3dL1tpwlnResJQR+NHPyTu7u6XrbThLOi9YSWAAP13AdVi8Xqe4VLs+URR2fUf81LdBdC0wcP/sRaAAKcF6uXyM88AoR/IygHdNUBL/8aQC7jOX6aXAOyBc1K2VQ+CPyDp09nbHKYJkoS95ltNPTkGdZFlnr+0URtVLKWlmoBjRoJBnLkxgPBan+q/7v0sFaAgqUTPVARxi0KeBvyE2fYvtN0wXfsYpll2ZyTFaWv74g+NvwOIorfxppmtw1Aoe44JrkNfy92+kxmHl09JB5IWndyUZRPg0ENUORNjjrfwR53QUrAv0Pbw3+KzCsmRClX/6j5qHi+MfbVkmbixgheNw1TRP+3vwHXXuzEZM9TBO9XANE2AzPm878LDSXDOwItzSeQesOJ62rm1UQvWaVDgH2Kn7M+8xwg+T/+OjC+iY095CFjS9okvc+qsvdTPVwAlO7LtBI0EIPzRzF5Gp/c/CFXvKUexvfh0wdgpZaH/mhYsOOYQ1+soKHvHXb8NNEASvkpeRQQ4cgkQeZ/n+265EAlN9vhsykU8uXhZMStw7o1Idw9eCuibOJ1q9fsUHxPsvdcdyKs6wWk+nD/AQNd6QoTqJzEJdsktuk7OmIPUMoHaVM8kQapQgGTe7ZzbSoFUgTNBpbI/v54p/QDvwLZtdANku5UFSccarNnogJ7N+vS0JAcldFM4Xktoll84uqcC+2xZIBAGyrliTzRiLwm2CMjXgif2ln3Kjoyhg7+VIHP5u6jkHupENMNqzDQCwxlAIZ/mZp6PTaakjeTLsEPPPQlP+HapL566KF1CLCjgDgyWkOPLlDPmi8UrpWJM4p9ujO2RAd5UT9KuIOrTRPTaFnL/Z3OmIwwnw9UKaRMXJyeGQL01KGd2+xR8WYSiFsv70JlpBI3+srAMzPIUll3LDCN7HfKyg9b34wav8xZWi8KcRgfRmRUtZ1y9JCMD+sTeve2bR0WxgWADnPLeEHRzPVh2b/invd69EQjklxr6vYcUllQuKf2NXXbpTXnPgBlUX+mCJFcAhtrgHCTQggsxeQT14ylsCgRtMvJHskl979QMxjLIwAF44RAQMYccBwblWgDiDoAAAAA="; $data = file_put_contents($uploaded_filename, base64_decode($imagestring)); - $_FILES = - array($fileId=>array( - "name"=>"testimage.png", - "type"=> "image/png", - "tmp_name"=>$uploaded_filename, - "error"=>0, - "size"=>1166 - )); + $_FILES = [ $fileId => [ + "name"=>"testimage.png", + "type"=> "image/png", + "tmp_name"=>$uploaded_filename, + "error"=>0, + "size"=>1166 + ]]; $this->_model = new Uploader($fileId); $this->assertEquals( file_exists($uploaded_filename), - true); + true + ); - unset($this->_model); } /** @@ -126,29 +115,28 @@ public function testFileMultipleUpload() $data = file_put_contents($uploaded_filename, base64_decode($imagestring)); $_FILES = - array($fileId=>array( - "name"=>array("testimage.png",""), - "type"=>array("image/png",""), - "tmp_name"=>array($uploaded_filename,""), - "error"=>array(0,4), - "size"=>array(1166,0) - )); - - $fileId=array( + [$fileId=>[ + "name"=>["testimage.png",""], + "type"=>["image/png",""], + "tmp_name"=>[$uploaded_filename,""], + "error"=>[0,4], + "size"=>[1166,0] + ]]; + + $fileId=[ "name"=>"testimage.png", "type"=> "image/png", "tmp_name"=>$uploaded_filename, "error"=>0, "size"=>1166 - ); + ]; $this->_model = new Uploader($fileId); $this->assertEquals( file_exists($uploaded_filename), - true); + true + ); - unset($this->_model); } - }