Skip to content

Commit 18fca45

Browse files
author
Joan He
committed
Merge remote-tracking branch 'arcticfoxes/MC-15385' into 2.3-qwerty-pr
2 parents 4b1d6c9 + d38b533 commit 18fca45

File tree

5 files changed

+176
-6
lines changed

5 files changed

+176
-6
lines changed

‎app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ public function getFilesCollection($path, $type = null)
328328
$item->setName($item->getBasename());
329329
$item->setShortName($this->_cmsWysiwygImages->getShortFilename($item->getBasename()));
330330
$item->setUrl($this->_cmsWysiwygImages->getCurrentUrl() . $item->getBasename());
331+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
331332
$item->setSize(filesize($item->getFilename()));
332333
$item->setMimeType(\mime_content_type($item->getFilename()));
333334

@@ -338,6 +339,7 @@ public function getFilesCollection($path, $type = null)
338339
$thumbUrl = $this->_backendUrl->getUrl('cms/*/thumbnail', ['file' => $item->getId()]);
339340
}
340341

342+
// phpcs:ignore Generic.PHP.NoSilencedErrors
341343
$size = @getimagesize($item->getFilename());
342344

343345
if (is_array($size)) {
@@ -413,6 +415,7 @@ public function createDirectory($name, $path)
413415
'id' => $this->_cmsWysiwygImages->convertPathToId($newPath),
414416
];
415417
return $result;
418+
// phpcs:ignore Magento2.Exceptions.ThrowCatch
416419
} catch (\Magento\Framework\Exception\FileSystemException $e) {
417420
throw new \Magento\Framework\Exception\LocalizedException(__('We cannot create a new directory.'));
418421
}
@@ -421,7 +424,7 @@ public function createDirectory($name, $path)
421424
/**
422425
* Recursively delete directory from storage
423426
*
424-
* @param string $path Target dir
427+
* @param string $path Absolute path to target directory
425428
* @return void
426429
* @throws \Magento\Framework\Exception\LocalizedException
427430
*/
@@ -430,12 +433,20 @@ public function deleteDirectory($path)
430433
if ($this->_coreFileStorageDb->checkDbUsage()) {
431434
$this->_directoryDatabaseFactory->create()->deleteDirectory($path);
432435
}
436+
if (!$this->isPathAllowed($path, $this->getConditionsForExcludeDirs())) {
437+
throw new \Magento\Framework\Exception\LocalizedException(
438+
__('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
439+
);
440+
}
433441
try {
434442
$this->_deleteByPath($path);
435443
$path = $this->getThumbnailRoot() . $this->_getRelativePathToRoot($path);
436444
$this->_deleteByPath($path);
445+
// phpcs:ignore Magento2.Exceptions.ThrowCatch
437446
} catch (\Magento\Framework\Exception\FileSystemException $e) {
438-
throw new \Magento\Framework\Exception\LocalizedException(__('We cannot delete directory %1.', $path));
447+
throw new \Magento\Framework\Exception\LocalizedException(
448+
__('We cannot delete directory %1.', $this->_getRelativePathToRoot($path))
449+
);
439450
}
440451
}
441452

@@ -482,13 +493,18 @@ public function deleteFile($target)
482493
/**
483494
* Upload and resize new file
484495
*
485-
* @param string $targetPath Target directory
496+
* @param string $targetPath Absolute path to target directory
486497
* @param string $type Type of storage, e.g. image, media etc.
487498
* @return array File info Array
488499
* @throws \Magento\Framework\Exception\LocalizedException
489500
*/
490501
public function uploadFile($targetPath, $type = null)
491502
{
503+
if (!$this->isPathAllowed($targetPath, $this->getConditionsForExcludeDirs())) {
504+
throw new \Magento\Framework\Exception\LocalizedException(
505+
__('We can\'t upload the file to current folder right now. Please try another folder.')
506+
);
507+
}
492508
/** @var \Magento\MediaStorage\Model\File\Uploader $uploader */
493509
$uploader = $this->_uploaderFactory->create(['fileId' => 'image']);
494510
$allowed = $this->getAllowedExtensions($type);
@@ -589,6 +605,7 @@ public function resizeFile($source, $keepRatio = true)
589605
$image->open($source);
590606
$image->keepAspectRatio($keepRatio);
591607
$image->resize($this->_resizeParameters['width'], $this->_resizeParameters['height']);
608+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
592609
$dest = $targetDir . '/' . pathinfo($source, PATHINFO_BASENAME);
593610
$image->save($dest);
594611
if ($this->_directory->isFile($this->_directory->getRelativePath($dest))) {
@@ -624,6 +641,7 @@ public function getThumbsPath($filePath = false)
624641
$thumbnailDir = $this->getThumbnailRoot();
625642

626643
if ($filePath && strpos($filePath, $mediaRootDir) === 0) {
644+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
627645
$thumbnailDir .= dirname(substr($filePath, strlen($mediaRootDir)));
628646
}
629647

@@ -674,6 +692,7 @@ public function isImage($filename)
674692
if (!$this->hasData('_image_extensions')) {
675693
$this->setData('_image_extensions', $this->getAllowedExtensions('image'));
676694
}
695+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
677696
$ext = strtolower(pathinfo($filename, PATHINFO_EXTENSION));
678697
return in_array($ext, $this->_getData('_image_extensions'));
679698
}
@@ -784,4 +803,29 @@ private function getExtensionsList($type = null): array
784803

785804
return $allowed;
786805
}
806+
807+
/**
808+
* Check if path is not in excluded dirs.
809+
*
810+
* @param string $path Absolute path
811+
* @param array $conditions Exclude conditions
812+
* @return bool
813+
*/
814+
private function isPathAllowed($path, array $conditions): bool
815+
{
816+
$isAllowed = true;
817+
$regExp = $conditions['reg_exp'] ? '~' . implode('|', array_keys($conditions['reg_exp'])) . '~i' : null;
818+
$storageRoot = $this->_cmsWysiwygImages->getStorageRoot();
819+
$storageRootLength = strlen($storageRoot);
820+
821+
$mediaSubPathname = substr($path, $storageRootLength);
822+
$rootChildParts = explode('/', '/' . ltrim($mediaSubPathname, '/'));
823+
824+
if (array_key_exists($rootChildParts[1], $conditions['plain'])
825+
|| ($regExp && preg_match($regExp, $path))) {
826+
$isAllowed = false;
827+
}
828+
829+
return $isAllowed;
830+
}
787831
}

‎app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class StorageTest extends \PHPUnit\Framework\TestCase
1818
/**
1919
* Directory paths samples
2020
*/
21-
const STORAGE_ROOT_DIR = '/storage/root/dir';
21+
const STORAGE_ROOT_DIR = '/storage/root/dir/';
2222

2323
const INVALID_DIRECTORY_OVER_ROOT = '/storage/some/another/dir';
2424

@@ -437,10 +437,11 @@ protected function generalTestGetDirsCollection($path, $collectionArray = [], $e
437437

438438
public function testUploadFile()
439439
{
440-
$targetPath = '/target/path';
440+
$path = 'target/path';
441+
$targetPath = self::STORAGE_ROOT_DIR . $path;
441442
$fileName = 'image.gif';
442443
$realPath = $targetPath . '/' . $fileName;
443-
$thumbnailTargetPath = self::STORAGE_ROOT_DIR . '/.thumbs';
444+
$thumbnailTargetPath = self::STORAGE_ROOT_DIR . '/.thumbs' . $path;
444445
$thumbnailDestination = $thumbnailTargetPath . '/' . $fileName;
445446
$type = 'image';
446447
$result = [

‎dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolderTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images;
88

99
use Magento\Framework\App\Filesystem\DirectoryList;
10+
use Magento\Framework\App\Response\HttpFactory as ResponseFactory;
1011

1112
/**
1213
* Test for \Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder class.
@@ -38,6 +39,11 @@ class DeleteFolderTest extends \PHPUnit\Framework\TestCase
3839
*/
3940
private $filesystem;
4041

42+
/**
43+
* @var HttpFactory
44+
*/
45+
private $responseFactory;
46+
4147
/**
4248
* @inheritdoc
4349
*/
@@ -49,6 +55,7 @@ protected function setUp()
4955
/** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */
5056
$this->imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class);
5157
$this->fullDirectoryPath = $this->imagesHelper->getStorageRoot();
58+
$this->responseFactory = $objectManager->get(ResponseFactory::class);
5259
$this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFolder::class);
5360
}
5461

@@ -83,6 +90,7 @@ public function testExecute()
8390
* can be removed.
8491
*
8592
* @magentoDataFixture Magento/Cms/_files/linked_media.php
93+
* @magentoAppIsolation enabled
8694
*/
8795
public function testExecuteWithLinkedMedia()
8896
{
@@ -106,6 +114,7 @@ public function testExecuteWithLinkedMedia()
106114
* under media directory.
107115
*
108116
* @return void
117+
* @magentoAppIsolation enabled
109118
*/
110119
public function testExecuteWithWrongDirectoryName()
111120
{
@@ -116,6 +125,31 @@ public function testExecuteWithWrongDirectoryName()
116125
$this->assertFileExists($this->fullDirectoryPath . $directoryName);
117126
}
118127

128+
/**
129+
* Execute method to check that there is no ability to remove folder which is in excluded directories list.
130+
*
131+
* @return void
132+
* @magentoAppIsolation enabled
133+
*/
134+
public function testExecuteWithExcludedDirectoryName()
135+
{
136+
$directoryName = 'downloadable';
137+
$expectedResponseMessage = 'We cannot delete directory /downloadable.';
138+
$mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA);
139+
$mediaDirectory->create($directoryName);
140+
$this->assertFileExists($this->fullDirectoryPath . $directoryName);
141+
142+
$this->model->getRequest()->setParams(['node' => $this->imagesHelper->idEncode($directoryName)]);
143+
$this->model->getRequest()->setMethod('POST');
144+
$jsonResponse = $this->model->execute();
145+
$jsonResponse->renderResult($response = $this->responseFactory->create());
146+
$data = json_decode($response->getBody(), true);
147+
148+
$this->assertTrue($data['error']);
149+
$this->assertEquals($expectedResponseMessage, $data['message']);
150+
$this->assertFileExists($this->fullDirectoryPath . $directoryName);
151+
}
152+
119153
/**
120154
* @inheritdoc
121155
*/
@@ -128,5 +162,8 @@ public static function tearDownAfterClass()
128162
if ($directory->isExist('wysiwyg')) {
129163
$directory->delete('wysiwyg');
130164
}
165+
if ($directory->isExist('downloadable')) {
166+
$directory->delete('downloadable');
167+
}
131168
}
132169
}

‎dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/UploadTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ class UploadTest extends \PHPUnit\Framework\TestCase
3333
*/
3434
private $fullDirectoryPath;
3535

36+
/**
37+
* @var string
38+
*/
39+
private $fullExcludedDirectoryPath;
40+
3641
/**
3742
* @var string
3843
*/
@@ -60,11 +65,13 @@ protected function setUp()
6065
{
6166
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
6267
$directoryName = 'directory1';
68+
$excludedDirName = 'downloadable';
6369
$this->filesystem = $this->objectManager->get(\Magento\Framework\Filesystem::class);
6470
/** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */
6571
$imagesHelper = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class);
6672
$this->mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA);
6773
$this->fullDirectoryPath = $imagesHelper->getStorageRoot() . DIRECTORY_SEPARATOR . $directoryName;
74+
$this->fullExcludedDirectoryPath = $imagesHelper->getStorageRoot() . DIRECTORY_SEPARATOR . $excludedDirName;
6875
$this->mediaDirectory->create($this->mediaDirectory->getRelativePath($this->fullDirectoryPath));
6976
$this->responseFactory = $this->objectManager->get(ResponseFactory::class);
7077
$this->model = $this->objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\Upload::class);
@@ -115,6 +122,34 @@ public function testExecute()
115122
$this->assertEquals($keys, $dataKeys);
116123
}
117124

125+
/**
126+
* Execute method with excluded directory path and file name to check that file can't be uploaded.
127+
*
128+
* @return void
129+
* @magentoAppIsolation enabled
130+
*/
131+
public function testExecuteWithExcludedDirectory()
132+
{
133+
$expectedError = 'We can\'t upload the file to current folder right now. Please try another folder.';
134+
$this->model->getRequest()->setParams(['type' => 'image/png']);
135+
$this->model->getRequest()->setMethod('POST');
136+
$this->model->getStorage()->getSession()->setCurrentPath($this->fullExcludedDirectoryPath);
137+
/** @var JsonResponse $jsonResponse */
138+
$jsonResponse = $this->model->execute();
139+
/** @var Response $response */
140+
$jsonResponse->renderResult($response = $this->responseFactory->create());
141+
$data = json_decode($response->getBody(), true);
142+
143+
$this->assertEquals($expectedError, $data['error']);
144+
$this->assertFalse(
145+
$this->mediaDirectory->isExist(
146+
$this->mediaDirectory->getRelativePath(
147+
$this->fullExcludedDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName
148+
)
149+
)
150+
);
151+
}
152+
118153
/**
119154
* Execute method with correct directory path and file name to check that file can be uploaded to the directory
120155
* located under linked folder.

‎dev/tests/integration/testsuite/Magento/Cms/Model/Wysiwyg/Images/StorageTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,31 @@ public function testGetThumbsPath(): void
107107
);
108108
}
109109

110+
/**
111+
* @return void
112+
*/
113+
public function testDeleteDirectory(): void
114+
{
115+
$path = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath();
116+
$dir = 'testDeleteDirectory';
117+
$fullPath = $path . $dir;
118+
$this->storage->createDirectory($dir, $path);
119+
$this->assertFileExists($fullPath);
120+
$this->storage->deleteDirectory($fullPath);
121+
$this->assertFileNotExists($fullPath);
122+
}
123+
124+
/**
125+
* @return void
126+
* @expectedException \Magento\Framework\Exception\LocalizedException
127+
* @expectedExceptionMessage We cannot delete directory /downloadable.
128+
*/
129+
public function testDeleteDirectoryWithExcludedDirPath(): void
130+
{
131+
$dir = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath() . 'downloadable';
132+
$this->storage->deleteDirectory($dir);
133+
}
134+
110135
/**
111136
* @return void
112137
*/
@@ -126,11 +151,39 @@ public function testUploadFile(): void
126151
'error' => 0,
127152
'size' => 12500,
128153
];
154+
129155
$this->storage->uploadFile(self::$_baseDir);
130156
$this->assertTrue(is_file(self::$_baseDir . DIRECTORY_SEPARATOR . $fileName));
131157
// phpcs:enable
132158
}
133159

160+
/**
161+
* @return void
162+
* @expectedException \Magento\Framework\Exception\LocalizedException
163+
* @expectedExceptionMessage We can't upload the file to current folder right now. Please try another folder.
164+
*/
165+
public function testUploadFileWithExcludedDirPath(): void
166+
{
167+
$fileName = 'magento_small_image.jpg';
168+
$tmpDirectory = $this->filesystem->getDirectoryWrite(\Magento\Framework\App\Filesystem\DirectoryList::SYS_TMP);
169+
$filePath = $tmpDirectory->getAbsolutePath($fileName);
170+
// phpcs:disable
171+
$fixtureDir = realpath(__DIR__ . '/../../../../Catalog/_files');
172+
copy($fixtureDir . DIRECTORY_SEPARATOR . $fileName, $filePath);
173+
174+
$_FILES['image'] = [
175+
'name' => $fileName,
176+
'type' => 'image/jpeg',
177+
'tmp_name' => $filePath,
178+
'error' => 0,
179+
'size' => 12500,
180+
];
181+
182+
$dir = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class)->getCurrentPath() . 'downloadable';
183+
$this->storage->uploadFile($dir);
184+
// phpcs:enable
185+
}
186+
134187
/**
135188
* @param string $fileName
136189
* @param string $fileType

0 commit comments

Comments
 (0)