Skip to content

Commit fad3a6d

Browse files
zakdmazakdma
committed
Merge remote-tracking branch 'origin/MC-18100' into 2.2.10-develop-pr106
2 parents d4d2597 + ef5c8ed commit fad3a6d

File tree

4 files changed

+249
-12
lines changed

4 files changed

+249
-12
lines changed

‎app/code/Magento/Downloadable/Controller/Adminhtml/Downloadable/File/Upload.php

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
use Magento\MediaStorage\Helper\File\Storage\Database;
1818
use Magento\Framework\App\RequestInterface;
1919
use Magento\Framework\App\State;
20+
use Magento\Framework\Exception\FileSystemException;
21+
use Magento\Framework\Exception\LocalizedException;
2022

2123
/**
2224
* Upload controller
@@ -109,23 +111,27 @@ public function dispatch(RequestInterface $request)
109111
*/
110112
public function execute()
111113
{
112-
$type = $this->getRequest()->getParam('type');
113-
$tmpPath = '';
114-
if ($type == 'samples') {
115-
$tmpPath = $this->_sample->getBaseTmpPath();
116-
} elseif ($type == 'links') {
117-
$tmpPath = $this->_link->getBaseTmpPath();
118-
} elseif ($type == 'link_samples') {
119-
$tmpPath = $this->_link->getBaseSampleTmpPath();
120-
}
121-
122114
try {
115+
$type = $this->getRequest()->getParam('type');
116+
$tmpPath = '';
117+
if ($type === 'samples') {
118+
$tmpPath = $this->_sample->getBaseTmpPath();
119+
} elseif ($type === 'links') {
120+
$tmpPath = $this->_link->getBaseTmpPath();
121+
} elseif ($type === 'link_samples') {
122+
$tmpPath = $this->_link->getBaseSampleTmpPath();
123+
} else {
124+
throw new LocalizedException(__('Upload type can not be determined.'));
125+
}
126+
123127
$uploader = $this->uploaderFactory->create(['fileId' => $type]);
124128

125129
$result = $this->_fileHelper->uploadFromTmp($tmpPath, $uploader);
126130

127131
if (!$result) {
128-
throw new \Exception('File can not be moved from temporary folder to the destination folder.');
132+
throw new FileSystemException(
133+
__('File can not be moved from temporary folder to the destination folder.')
134+
);
129135
}
130136

131137
unset($result['tmp_name'], $result['path']);
@@ -137,6 +143,7 @@ public function execute()
137143
} catch (\Exception $e) {
138144
$result = ['error' => $e->getMessage(), 'errorcode' => $e->getCode()];
139145
}
146+
140147
return $this->resultFactory->create(ResultFactory::TYPE_JSON)->setData($result);
141148
}
142149
}

‎dev/tests/integration/testsuite/Magento/Downloadable/Controller/Adminhtml/Downloadable/File/UploadTest.php

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Downloadable\Controller\Adminhtml\Downloadable\File;
9+
10+
use Magento\Framework\Serialize\Serializer\Json;
11+
12+
/**
13+
* Test for Magento\Downloadable\Controller\Adminhtml\Downloadable\File\Upload
14+
*
15+
* @magentoAppArea adminhtml
16+
*/
17+
class UploadTest extends \Magento\TestFramework\TestCase\AbstractBackendController
18+
{
19+
/**
20+
* @var Json
21+
*/
22+
private $jsonSerializer;
23+
24+
/**
25+
* @inheritdoc
26+
*/
27+
protected function setUp()
28+
{
29+
parent::setUp();
30+
31+
$this->jsonSerializer = $this->_objectManager->get(Json::class);
32+
}
33+
34+
/**
35+
* @dataProvider uploadWrongUploadTypeDataProvider
36+
* @return void
37+
*/
38+
public function testUploadWrongUploadType($postData)
39+
{
40+
$this->getRequest()->setPostValue($postData);
41+
$this->getRequest()->setMethod('POST');
42+
43+
$this->dispatch('backend/admin/downloadable_file/upload');
44+
45+
$body = $this->getResponse()->getBody();
46+
$result = $this->jsonSerializer->unserialize($body);
47+
$this->assertEquals('Upload type can not be determined.', $result['error']);
48+
$this->assertEquals(0, $result['errorcode']);
49+
}
50+
51+
/**
52+
* @return array
53+
*/
54+
public function uploadWrongUploadTypeDataProvider(): array
55+
{
56+
return [
57+
[
58+
['type' => 'test'],
59+
],
60+
[
61+
[
62+
'type' => [
63+
'type1' => 'test',
64+
],
65+
],
66+
],
67+
];
68+
}
69+
}

‎dev/tests/integration/testsuite/Magento/Framework/File/UploaderTest.php

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\File;
9+
10+
use Magento\Framework\App\Filesystem\DirectoryList;
11+
12+
/**
13+
* Test for \Magento\Framework\File\Uploader
14+
*/
15+
class UploaderTest extends \PHPUnit\Framework\TestCase
16+
{
17+
/**
18+
* @var \Magento\MediaStorage\Model\File\UploaderFactory
19+
*/
20+
private $uploaderFactory;
21+
22+
/**
23+
* @var \Magento\Framework\Filesystem
24+
*/
25+
private $filesystem;
26+
27+
/**
28+
* @inheritdoc
29+
*/
30+
protected function setUp()
31+
{
32+
$this->uploaderFactory = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
33+
->get(\Magento\MediaStorage\Model\File\UploaderFactory::class);
34+
35+
$this->filesystem = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
36+
->get(\Magento\Framework\Filesystem::class);
37+
}
38+
39+
/**
40+
* @return void
41+
*/
42+
public function testUploadFileFromAllowedFolder()
43+
{
44+
$mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA);
45+
$tmpDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::SYS_TMP);
46+
47+
$fileName = 'text.txt';
48+
$tmpDir = 'tmp';
49+
$filePath = $tmpDirectory->getAbsolutePath($fileName);
50+
51+
$tmpDirectory->writeFile($fileName, 'just a text');
52+
53+
$type = [
54+
'tmp_name' => $filePath,
55+
'name' => $fileName,
56+
];
57+
58+
$uploader = $this->uploaderFactory->create(['fileId' => $type]);
59+
$uploader->save($mediaDirectory->getAbsolutePath($tmpDir));
60+
61+
$this->assertTrue($mediaDirectory->isFile($tmpDir . DIRECTORY_SEPARATOR . $fileName));
62+
}
63+
64+
/**
65+
* @expectedException \InvalidArgumentException
66+
* @expectedExceptionMessage Invalid parameter given. A valid $fileId[tmp_name] is expected.
67+
*
68+
* @return void
69+
*/
70+
public function testUploadFileFromNotAllowedFolder()
71+
{
72+
$fileName = 'text.txt';
73+
$tmpDir = 'tmp';
74+
$tmpDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::LOG);
75+
$filePath = $tmpDirectory->getAbsolutePath() . $tmpDir . DIRECTORY_SEPARATOR . $fileName;
76+
77+
$tmpDirectory->writeFile($tmpDir . DIRECTORY_SEPARATOR . $fileName, 'just a text');
78+
79+
$type = [
80+
'tmp_name' => $filePath,
81+
'name' => $fileName,
82+
];
83+
84+
$this->uploaderFactory->create(['fileId' => $type]);
85+
}
86+
87+
/**
88+
* @inheritdoc
89+
*/
90+
protected function tearDown()
91+
{
92+
$tmpDir = 'tmp';
93+
$mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA);
94+
$mediaDirectory->delete($tmpDir);
95+
96+
$logDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::LOG);
97+
$logDirectory->delete($tmpDir);
98+
}
99+
}

‎lib/internal/Magento/Framework/File/Uploader.php

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace Magento\Framework\File;
77

8+
use Magento\Framework\App\Filesystem\DirectoryList;
89
use Magento\Framework\Exception\FileSystemException;
910
use Magento\Framework\Validation\ValidationException;
1011

@@ -14,6 +15,8 @@
1415
* ATTENTION! This class must be used like abstract class and must added
1516
* validation by protected file extension list to extended class
1617
*
18+
* @SuppressWarnings(PHPMD.TooManyFields)
19+
*
1720
* @api
1821
*/
1922
class Uploader
@@ -160,16 +163,23 @@ class Uploader
160163
*/
161164
protected $_result;
162165

166+
/**
167+
* @var DirectoryList
168+
*/
169+
private $directoryList;
170+
163171
/**
164172
* Init upload
165173
*
166174
* @param string|array $fileId
167175
* @param \Magento\Framework\File\Mime|null $fileMime
176+
* @param DirectoryList|null $directoryList
168177
* @throws \DomainException
169178
*/
170179
public function __construct(
171180
$fileId,
172-
Mime $fileMime = null
181+
Mime $fileMime = null,
182+
DirectoryList $directoryList = null
173183
) {
174184
$this->_setUploadFileId($fileId);
175185
if (!file_exists($this->_file['tmp_name'])) {
@@ -179,6 +189,8 @@ public function __construct(
179189
$this->_fileExists = true;
180190
}
181191
$this->fileMime = $fileMime ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Mime::class);
192+
$this->directoryList= $directoryList ?: \Magento\Framework\App\ObjectManager::getInstance()
193+
->get(DirectoryList::class);
182194
}
183195

184196
/**
@@ -552,6 +564,7 @@ private function _getMimeType()
552564
private function _setUploadFileId($fileId)
553565
{
554566
if (is_array($fileId)) {
567+
$this->validateFileId($fileId);
555568
$this->_uploadType = self::MULTIPLE_STYLE;
556569
$this->_file = $fileId;
557570
} else {
@@ -585,6 +598,55 @@ private function _setUploadFileId($fileId)
585598
}
586599
}
587600

601+
/**
602+
* Validates explicitly given uploaded file data.
603+
*
604+
* @param array $fileId
605+
* @return void
606+
* @throws \InvalidArgumentException
607+
*/
608+
private function validateFileId(array $fileId)
609+
{
610+
$isValid = false;
611+
if (isset($fileId['tmp_name'])) {
612+
$tmpName = trim($fileId['tmp_name']);
613+
614+
if (preg_match('/\.\.(\\\|\/)/', $tmpName) !== 1) {
615+
$allowedFolders = [
616+
sys_get_temp_dir(),
617+
$this->directoryList->getPath(DirectoryList::MEDIA),
618+
$this->directoryList->getPath(DirectoryList::VAR_DIR),
619+
$this->directoryList->getPath(DirectoryList::TMP),
620+
$this->directoryList->getPath(DirectoryList::UPLOAD),
621+
];
622+
623+
$disallowedFolders = [
624+
$this->directoryList->getPath(DirectoryList::LOG),
625+
];
626+
627+
foreach ($allowedFolders as $allowedFolder) {
628+
if (stripos($tmpName, $allowedFolder) === 0) {
629+
$isValid = true;
630+
break;
631+
}
632+
}
633+
634+
foreach ($disallowedFolders as $disallowedFolder) {
635+
if (stripos($tmpName, $disallowedFolder) === 0) {
636+
$isValid = false;
637+
break;
638+
}
639+
}
640+
}
641+
}
642+
643+
if (!$isValid) {
644+
throw new \InvalidArgumentException(
645+
__('Invalid parameter given. A valid $fileId[tmp_name] is expected.')
646+
);
647+
}
648+
}
649+
588650
/**
589651
* Create destination folder
590652
*

0 commit comments

Comments
 (0)