diff --git a/app/code/Magento/Cms/Helper/Wysiwyg/Images.php b/app/code/Magento/Cms/Helper/Wysiwyg/Images.php index 18c7d14cc8fcf..58f074e72200b 100644 --- a/app/code/Magento/Cms/Helper/Wysiwyg/Images.php +++ b/app/code/Magento/Cms/Helper/Wysiwyg/Images.php @@ -27,11 +27,11 @@ class Images extends \Magento\Framework\App\Helper\AbstractHelper protected $_currentUrl; /** - * Currenty selected store ID if applicable + * Currently selected store ID if applicable * * @var int */ - protected $_storeId = null; + protected $_storeId; /** * @var \Magento\Framework\Filesystem\Directory\Write @@ -71,7 +71,7 @@ public function __construct( $this->_storeManager = $storeManager; $this->_directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA); - $this->_directory->create(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY); + $this->_directory->create($this->getStorageRoot()); } /** @@ -93,7 +93,17 @@ public function setStoreId($store) */ public function getStorageRoot() { - return $this->_directory->getAbsolutePath(\Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY); + return $this->_directory->getAbsolutePath($this->getStorageRootSubpath()); + } + + /** + * Get image storage root subpath. User is unable to traverse outside of this subpath in media gallery + * + * @return string + */ + public function getStorageRootSubpath() + { + return ''; } /** @@ -141,7 +151,7 @@ public function convertIdToPath($id) return $this->getStorageRoot(); } else { $path = $this->getStorageRoot() . $this->idDecode($id); - if (strpos($path, DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR) !== false) { + if (preg_match('/\.\.(\\\|\/)/', $path)) { throw new \InvalidArgumentException('Path is invalid'); } @@ -208,7 +218,7 @@ public function getImageHtmlDeclaration($filename, $renderAsTag = false) public function getCurrentPath() { if (!$this->_currentPath) { - $currentPath = $this->_directory->getAbsolutePath() . \Magento\Cms\Model\Wysiwyg\Config::IMAGE_DIRECTORY; + $currentPath = $this->getStorageRoot(); $path = $this->_getRequest()->getParam($this->getTreeNodeName()); if ($path) { $path = $this->convertIdToPath($path); @@ -244,7 +254,7 @@ public function getCurrentUrl() )->getBaseUrl( \Magento\Framework\UrlInterface::URL_TYPE_MEDIA ); - $this->_currentUrl = $mediaUrl . $this->_directory->getRelativePath($path) . '/'; + $this->_currentUrl = rtrim($mediaUrl . $this->_directory->getRelativePath($path), '/') . '/'; } return $this->_currentUrl; } @@ -261,7 +271,7 @@ public function idEncode($string) } /** - * Revert opration to idEncode + * Revert operation to idEncode * * @param string $string * @return string diff --git a/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php b/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php index 90dcf3dc8df78..ec099efb8e5a3 100644 --- a/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php +++ b/app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php @@ -243,10 +243,12 @@ protected function getConditionsForExcludeDirs() protected function removeItemFromCollection($collection, $conditions) { $regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null; - $storageRootLength = strlen($this->_cmsWysiwygImages->getStorageRoot()); + $storageRoot = $this->_cmsWysiwygImages->getStorageRoot(); + $storageRootLength = strlen($storageRoot); foreach ($collection as $key => $value) { - $rootChildParts = explode('/', substr($value->getFilename(), $storageRootLength)); + $mediaSubPathname = substr($value->getFilename(), $storageRootLength); + $rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/')); if (array_key_exists($rootChildParts[1], $conditions['plain']) || ($regExp && preg_match($regExp, $value->getFilename()))) { @@ -321,6 +323,8 @@ public function getFilesCollection($path, $type = null) $item->setName($item->getBasename()); $item->setShortName($this->_cmsWysiwygImages->getShortFilename($item->getBasename())); $item->setUrl($this->_cmsWysiwygImages->getCurrentUrl() . $item->getBasename()); + $item->setSize(filesize($item->getFilename())); + $item->setMimeType(\mime_content_type($item->getFilename())); if ($this->isImage($item->getBasename())) { $thumbUrl = $this->getThumbnailUrl($item->getFilename(), true); @@ -412,7 +416,7 @@ public function createDirectory($name, $path) /** * Recursively delete directory from storage * - * @param string $path Target dir + * @param string $path Absolute path to target directory * @return void * @throws \Magento\Framework\Exception\LocalizedException */ @@ -421,12 +425,19 @@ public function deleteDirectory($path) if ($this->_coreFileStorageDb->checkDbUsage()) { $this->_directoryDatabaseFactory->create()->deleteDirectory($path); } + if (!$this->isPathAllowed($path, $this->getConditionsForExcludeDirs())) { + throw new \Magento\Framework\Exception\LocalizedException( + __('We cannot delete directory %1.', $this->_getRelativePathToRoot($path)) + ); + } try { $this->_deleteByPath($path); $path = $this->getThumbnailRoot() . $this->_getRelativePathToRoot($path); $this->_deleteByPath($path); } catch (\Magento\Framework\Exception\FileSystemException $e) { - throw new \Magento\Framework\Exception\LocalizedException(__('We cannot delete directory %1.', $path)); + throw new \Magento\Framework\Exception\LocalizedException( + __('We cannot delete directory %1.', $this->_getRelativePathToRoot($path)) + ); } } @@ -473,14 +484,18 @@ public function deleteFile($target) /** * Upload and resize new file * - * @param string $targetPath Target directory + * @param string $targetPath Absolute path to target directory * @param string $type Type of storage, e.g. image, media etc. * @return array File info Array * @throws \Magento\Framework\Exception\LocalizedException - * @throws \Exception */ public function uploadFile($targetPath, $type = null) { + if (!$this->isPathAllowed($targetPath, $this->getConditionsForExcludeDirs())) { + throw new \Magento\Framework\Exception\LocalizedException( + __('We can\'t upload the file to current folder right now. Please try another folder.') + ); + } /** @var \Magento\MediaStorage\Model\File\Uploader $uploader */ $uploader = $this->_uploaderFactory->create(['fileId' => 'image']); $allowed = $this->getAllowedExtensions($type); @@ -748,7 +763,7 @@ protected function _getRelativePathToRoot($path) } /** - * Prepare mime types config settings + * Prepare mime types config settings. * * @param string|null $type Type of storage, e.g. image, media etc. * @return array Array of allowed file extensions @@ -761,7 +776,7 @@ private function getAllowedMimeTypes($type = null): array } /** - * Get list of allowed file extensions with mime type in values + * Get list of allowed file extensions with mime type in values. * * @param string|null $type * @return array @@ -775,4 +790,29 @@ private function getExtensionsList($type = null): array } return $allowed; } + + /** + * Check if path is not in excluded dirs. + * + * @param string $path Absolute path + * @param array $conditions Exclude conditions + * @return bool + */ + private function isPathAllowed($path, array $conditions): bool + { + $isAllowed = true; + $regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null; + $storageRoot = $this->_cmsWysiwygImages->getStorageRoot(); + $storageRootLength = strlen($storageRoot); + + $mediaSubPathname = substr($path, $storageRootLength); + $rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/')); + + if (array_key_exists($rootChildParts[1], $conditions['plain']) + || ($regExp && preg_match($regExp, $path))) { + $isAllowed = false; + } + + return $isAllowed; + } } diff --git a/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php b/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php index 20c0b2075f5c3..68c2a164b9047 100644 --- a/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php +++ b/app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php @@ -18,7 +18,7 @@ class StorageTest extends \PHPUnit\Framework\TestCase /** * Directory paths samples */ - const STORAGE_ROOT_DIR = '/storage/root/dir'; + const STORAGE_ROOT_DIR = '/storage/root/dir/'; const INVALID_DIRECTORY_OVER_ROOT = '/storage/some/another/dir'; @@ -434,10 +434,11 @@ protected function generalTestGetDirsCollection($path, $collectionArray = [], $e public function testUploadFile() { - $targetPath = '/target/path'; + $path = 'target/path'; + $targetPath = self::STORAGE_ROOT_DIR . $path; $fileName = 'image.gif'; $realPath = $targetPath . '/' . $fileName; - $thumbnailTargetPath = self::STORAGE_ROOT_DIR . '/.thumbs'; + $thumbnailTargetPath = self::STORAGE_ROOT_DIR . '/.thumbs' . $path; $thumbnailDestination = $thumbnailTargetPath . '/' . $fileName; $type = 'image'; $result = [ diff --git a/app/code/Magento/Cms/etc/di.xml b/app/code/Magento/Cms/etc/di.xml index 3512714742f2c..b883e8f93f752 100644 --- a/app/code/Magento/Cms/etc/di.xml +++ b/app/code/Magento/Cms/etc/di.xml @@ -51,8 +51,41 @@ - - + + + true + pub[/\\]+media[/\\]+captcha[/\\]*$ + + + true + pub[/\\]+media[/\\]+catalog[/\\]+product[/\\]*$ + + + true + pub[/\\]+media[/\\]+customer[/\\]*$ + + + true + pub[/\\]+media[/\\]+downloadable[/\\]*$ + + + true + pub[/\\]+media[/\\]+import[/\\]*$ + + + true + pub[/\\]+media[/\\]+theme[/\\]*$ + + + true + pub[/\\]+media[/\\]+theme_customization[/\\]*$ + + + true + pub[/\\]+media[/\\]+tmp[/\\]*$ + + + diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php index 994d4d1412b05..294a0490333f2 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php @@ -7,6 +7,7 @@ namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\App\Response\HttpFactory as ResponseFactory; /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder class. @@ -28,11 +29,21 @@ class DeleteFolderTest extends \PHPUnit\Framework\TestCase */ private $mediaDirectory; + /** + * @var string + */ + private $fullDirectoryPath; + /** * @var \Magento\Framework\Filesystem */ private $filesystem; + /** + * @var ResponseFactory + */ + private $responseFactory; + /** * @inheritdoc */ @@ -41,14 +52,18 @@ protected function setUp() $objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); $this->filesystem = $objectManager->get(\Magento\Framework\Filesystem::class); $this->mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA); - /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ $this->imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); + $this->fullDirectoryPath = $this->imagesHelper->getStorageRoot(); + $this->responseFactory = $objectManager->get(ResponseFactory::class); + /** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */ $this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder::class); } /** * Execute method with correct directory path to check that directories under WYSIWYG media directory * can be removed. + * + * @magentoAppIsolation enabled */ public function testExecute() { @@ -74,6 +89,7 @@ public function testExecute() * can be removed. * * @magentoDataFixture Magento/Cms/_files/linked_media.php + * @magentoAppIsolation enabled */ public function testExecuteWithLinkedMedia() { @@ -85,11 +101,39 @@ public function testExecuteWithLinkedMedia() $linkedDirectory->create( $linkedDirectory->getRelativePath($linkedDirectoryPath . DIRECTORY_SEPARATOR . $directoryName) ); - $this->model->getRequest()->setParams(['node' => $this->imagesHelper->idEncode($directoryName)]); + $this->model->getRequest()->setParams( + ['node' => $this->imagesHelper->idEncode('wysiwyg' . DIRECTORY_SEPARATOR . $directoryName)] + ); + $this->model->getRequest()->setMethod('POST'); $this->model->execute(); $this->assertFalse(is_dir($linkedDirectoryPath . DIRECTORY_SEPARATOR . $directoryName)); } + /** + * Execute method to check that there is no ability to remove folder which is in excluded directories list. + * + * @return void + * @magentoAppIsolation enabled + */ + public function testExecuteWithExcludedDirectoryName() + { + $directoryName = 'downloadable'; + $expectedResponseMessage = 'We cannot delete directory /downloadable.'; + $mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA); + $mediaDirectory->create($directoryName); + $this->assertFileExists($this->fullDirectoryPath . DIRECTORY_SEPARATOR . $directoryName); + + $this->model->getRequest()->setParams(['node' => $this->imagesHelper->idEncode($directoryName)]); + $this->model->getRequest()->setMethod('POST'); + $jsonResponse = $this->model->execute(); + $jsonResponse->renderResult($response = $this->responseFactory->create()); + $data = json_decode($response->getBody(), true); + + $this->assertTrue($data['error']); + $this->assertEquals($expectedResponseMessage, $data['message']); + $this->assertFileExists($this->fullDirectoryPath . $directoryName); + } + /** * @inheritdoc */ @@ -102,5 +146,8 @@ public static function tearDownAfterClass() if ($directory->isExist('wysiwyg')) { $directory->delete('wysiwyg'); } + if ($directory->isExist('downloadable')) { + $directory->delete('downloadable'); + } } } diff --git a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php index 6b1f8fc717c2d..c0fca0f675afe 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php @@ -7,6 +7,7 @@ namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images; use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\App\Response\HttpFactory as ResponseFactory; /** * Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\Upload class. @@ -23,6 +24,11 @@ class UploadTest extends \PHPUnit\Framework\TestCase */ private $mediaDirectory; + /** + * @var string + */ + private $fullExcludedDirectoryPath; + /** * @var string */ @@ -38,17 +44,26 @@ class UploadTest extends \PHPUnit\Framework\TestCase */ private $objectManager; + /** + * @var ResponseFactory + */ + private $responseFactory; + /** * @inheritdoc */ protected function setUp() { $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); + $excludedDirName = 'downloadable'; $this->filesystem = $this->objectManager->get(\Magento\Framework\Filesystem::class); + $imagesHelper = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class); $this->mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA); $this->model = $this->objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\Upload::class); $fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files'); $tmpFile = $this->filesystem->getDirectoryRead(DirectoryList::PUB)->getAbsolutePath() . $this->fileName; + $this->fullExcludedDirectoryPath = $imagesHelper->getStorageRoot() . DIRECTORY_SEPARATOR . $excludedDirName; + $this->responseFactory = $this->objectManager->get(ResponseFactory::class); copy($fixtureDir . DIRECTORY_SEPARATOR . $this->fileName, $tmpFile); $_FILES = [ 'image' => [ @@ -89,6 +104,34 @@ public function testExecute() ); } + /** + * Execute method with excluded directory path and file name to check that file can't be uploaded. + * + * @return void + * @magentoAppIsolation enabled + */ + public function testExecuteWithExcludedDirectory() + { + $expectedError = 'We can\'t upload the file to current folder right now. Please try another folder.'; + $this->model->getRequest()->setParams(['type' => 'image/png']); + $this->model->getRequest()->setMethod('POST'); + $this->model->getStorage()->getSession()->setCurrentPath($this->fullExcludedDirectoryPath); + /** @var JsonResponse $jsonResponse */ + $jsonResponse = $this->model->execute(); + /** @var Response $response */ + $jsonResponse->renderResult($response = $this->responseFactory->create()); + $data = json_decode($response->getBody(), true); + + $this->assertEquals($expectedError, $data['error']); + $this->assertFalse( + $this->mediaDirectory->isExist( + $this->mediaDirectory->getRelativePath( + $this->fullExcludedDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName + ) + ) + ); + } + /** * Execute method with correct directory path and file name to check that file can be uploaded to the directory * located under linked folder. diff --git a/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php b/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php index 71130a4e287f3..517d9748af786 100644 --- a/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php +++ b/dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php @@ -61,6 +61,31 @@ public static function tearDownAfterClass() ); } + /** + * @return void + */ + public function testDeleteDirectory(): void + { + $path = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath(); + $dir = 'testDeleteDirectory'; + $fullPath = $path . $dir; + $this->storage->createDirectory($dir, $path); + $this->assertFileExists($fullPath); + $this->storage->deleteDirectory($fullPath); + $this->assertFileNotExists($fullPath); + } + + /** + * @return void + * @expectedException \Magento\Framework\Exception\LocalizedException + * @expectedExceptionMessage We cannot delete directory /downloadable. + */ + public function testDeleteDirectoryWithExcludedDirPath(): void + { + $dir = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath() . 'downloadable'; + $this->storage->deleteDirectory($dir); + } + /** * @inheritdoc */ @@ -122,6 +147,31 @@ public function testUploadFile() $this->assertTrue(is_file(self::$_baseDir . DIRECTORY_SEPARATOR . $fileName)); } + /** + * @return void + * @expectedException \Magento\Framework\Exception\LocalizedException + * @expectedExceptionMessage We can't upload the file to current folder right now. Please try another folder. + */ + public function testUploadFileWithExcludedDirPath(): void + { + $fileName = 'magento_small_image.jpg'; + $tmpDirectory = $this->filesystem->getDirectoryWrite(\Magento\Framework\App\Filesystem\DirectoryList::SYS_TMP); + $filePath = $tmpDirectory->getAbsolutePath($fileName); + $fixtureDir = realpath(__DIR__ . '/../../../../Catalog/_files'); + copy($fixtureDir . DIRECTORY_SEPARATOR . $fileName, $filePath); + + $_FILES['image'] = [ + 'name' => $fileName, + 'type' => 'image/jpeg', + 'tmp_name' => $filePath, + 'error' => 0, + 'size' => 12500, + ]; + + $dir = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath() . 'downloadable'; + $this->storage->uploadFile($dir); + } + /** * @param string $fileName * @param string $fileType