Skip to content

Commit acd6ffc

Browse files
authored
Security changes from upstream 2.4.7-p2 (#101)
1 parent 1089058 commit acd6ffc

File tree

52 files changed

+469
-163
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+469
-163
lines changed

app/code/Magento/Catalog/Model/Product/Option/Type/File/ValidatorInfo.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class ValidatorInfo extends Validator
4949
* @var IoFile
5050
*/
5151
private $ioFile;
52+
5253
/**
5354
* @var NotProtectedExtension
5455
*/
@@ -147,12 +148,14 @@ private function validatePath(array $optionValuePath): bool
147148
{
148149
foreach ([$optionValuePath['quote_path'], $optionValuePath['order_path']] as $path) {
149150
$pathInfo = $this->ioFile->getPathInfo($path);
150-
if (isset($pathInfo['extension'])) {
151-
if (!$this->fileValidator->isValid($pathInfo['extension'])) {
152-
return false;
153-
}
151+
152+
if (isset($pathInfo['extension'])
153+
&& (empty($pathInfo['extension']) || !$this->fileValidator->isValid($pathInfo['extension']))
154+
) {
155+
return false;
154156
}
155157
}
158+
156159
return true;
157160
}
158161

app/code/Magento/Catalog/Test/Mftf/ActionGroup/StorefrontAssertActiveProductImageActionGroup.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@
1212
<arguments>
1313
<argument name="fileName" defaultValue="magento-logo" type="string"/>
1414
</arguments>
15-
<seeElement selector="{{StorefrontProductMediaSection.productImageActive(fileName)}}" stepKey="seeActiveImageDefault"/>
15+
<waitForElementVisible selector="{{StorefrontProductMediaSection.productImageActive(fileName)}}" stepKey="seeActiveImageDefault"/>
1616
</actionGroup>
1717
</actionGroups>

app/code/Magento/Catalog/Test/Mftf/Test/EndToEndB2CAdminTest.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@
216216
<comment userInput="Admin creates category." stepKey="adminCreatesCategoryComment" before="navigateToCategoryPage"/>
217217
<actionGroup ref="AdminOpenCategoryPageActionGroup" stepKey="navigateToCategoryPage"/>
218218
<!--Create category under Default Category-->
219-
<click selector="{{AdminCategorySidebarTreeSection.expandCategoryByName('Default Category')}}" stepKey="clickDefaultCategory"/>
219+
<click selector="{{AdminCategorySidebarTreeSection.categoryInTree(_defaultCategory.name)}}" stepKey="clickDefaultCategory"/>
220220
<actionGroup ref="CheckCategoryNameIsRequiredFieldActionGroup" stepKey="checkCategoryNameIsRequired"/>
221221
<actionGroup ref="CreateCategoryActionGroup" stepKey="createCategory">
222222
<argument name="categoryEntity" value="_defaultCategory"/>

app/code/Magento/Config/Model/Config/Backend/File.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,10 @@ protected function _getAllowedExtensions()
278278
*/
279279
private function setValueAfterValidation(string $value): void
280280
{
281-
// avoid intercepting value
282-
if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)) {
281+
if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)
282+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
283+
|| !$this->_mediaDirectory->isFile($this->_getUploadDir() . DIRECTORY_SEPARATOR . basename($value))
284+
) {
283285
throw new LocalizedException(__('Invalid file name'));
284286
}
285287

app/code/Magento/Customer/Model/Plugin/UpdateCustomer.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ public function beforeSave(
6363
$customerId === $customerSessionId
6464
) {
6565
$customer = $this->getUpdatedCustomer($customerRepository->getById($customerId), $customer);
66-
} elseif ($userType === UserContextInterface::USER_TYPE_ADMIN && $customerId) {
66+
} elseif ($customerId && in_array($userType, [UserContextInterface::USER_TYPE_ADMIN,
67+
UserContextInterface::USER_TYPE_INTEGRATION], true)
68+
) {
6769
$customer = $this->getUpdatedCustomer($customerRepository->getById($customerId), $customer);
6870
}
6971

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\EncryptionKey\Console\Command;
10+
11+
use Magento\Framework\App\DeploymentConfig\Writer;
12+
use Magento\Framework\Config\ConfigOptionsListConstants;
13+
use Magento\Framework\Config\Data\ConfigData;
14+
use Magento\Framework\Config\File\ConfigFilePool;
15+
use Magento\Framework\Exception\FileSystemException;
16+
use Magento\Framework\Math\Random;
17+
use Symfony\Component\Console\Command\Command;
18+
use Symfony\Component\Console\Input\InputInterface;
19+
use Symfony\Component\Console\Input\InputOption;
20+
use Symfony\Component\Console\Output\OutputInterface;
21+
use Magento\Framework\App\CacheInterface;
22+
use Magento\Framework\Encryption\EncryptorInterface;
23+
24+
class UpdateEncryptionKeyCommand extends Command
25+
{
26+
/**
27+
* @var EncryptorInterface
28+
*/
29+
private EncryptorInterface $encryptor;
30+
31+
/**
32+
* @var CacheInterface
33+
*/
34+
private CacheInterface $cache;
35+
36+
/**
37+
* Configuration writer
38+
*
39+
* @var Writer
40+
*/
41+
private Writer $writer;
42+
43+
/**
44+
* Random string generator
45+
*
46+
* @var Random
47+
*/
48+
private Random $random;
49+
50+
/**
51+
* @param EncryptorInterface $encryptor
52+
* @param CacheInterface $cache
53+
* @param Writer $writer
54+
* @param Random $random
55+
*/
56+
public function __construct(EncryptorInterface $encryptor, CacheInterface $cache, Writer $writer, Random $random)
57+
{
58+
$this->encryptor = $encryptor;
59+
$this->cache = $cache;
60+
$this->writer = $writer;
61+
$this->random = $random;
62+
63+
parent::__construct();
64+
}
65+
66+
/**
67+
* @inheritDoc
68+
*/
69+
protected function configure()
70+
{
71+
$this->setName('encryption:key:change');
72+
$this->setDescription('Change the encryption key inside the env.php file.');
73+
$this->addOption(
74+
'key',
75+
'k',
76+
InputOption::VALUE_OPTIONAL,
77+
'Key has to be a 32 characters long string. If not provided, a random key will be generated.'
78+
);
79+
80+
parent::configure();
81+
}
82+
83+
/**
84+
* @inheritDoc
85+
*/
86+
protected function execute(InputInterface $input, OutputInterface $output)
87+
{
88+
try {
89+
$key = $input->getOption('key');
90+
91+
if (!empty($key)) {
92+
$this->encryptor->validateKey($key);
93+
}
94+
95+
$this->updateEncryptionKey($key);
96+
$this->cache->clean();
97+
98+
$output->writeln('<info>Encryption key has been updated successfully.</info>');
99+
100+
return Command::SUCCESS;
101+
} catch (\Exception $e) {
102+
$output->writeln('<error>' . $e->getMessage() . '</error>');
103+
return Command::FAILURE;
104+
}
105+
}
106+
107+
/**
108+
* Update encryption key
109+
*
110+
* @param string|null $key
111+
* @return void
112+
* @throws FileSystemException
113+
*/
114+
private function updateEncryptionKey(string $key = null): void
115+
{
116+
// prepare new key, encryptor and new configuration segment
117+
if (!$this->writer->checkIfWritable()) {
118+
throw new FileSystemException(__('Deployment configuration file is not writable.'));
119+
}
120+
121+
if (null === $key) {
122+
$key = ConfigOptionsListConstants::STORE_KEY_ENCODED_RANDOM_STRING_PREFIX .
123+
$this->random->getRandomBytes(ConfigOptionsListConstants::STORE_KEY_RANDOM_STRING_SIZE);
124+
}
125+
126+
$this->encryptor->setNewKey($key);
127+
128+
$encryptSegment = new ConfigData(ConfigFilePool::APP_ENV);
129+
$encryptSegment->set(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, $this->encryptor->exportKeys());
130+
131+
$configData = [$encryptSegment->getFileKey() => $encryptSegment->getData()];
132+
133+
$this->writer->saveConfig($configData);
134+
}
135+
}

app/code/Magento/EncryptionKey/etc/di.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,11 @@
1111
<argument name="structure" xsi:type="object">Magento\Config\Model\Config\Structure\Proxy</argument>
1212
</arguments>
1313
</type>
14+
<type name="Magento\Framework\Console\CommandList">
15+
<arguments>
16+
<argument name="commands" xsi:type="array">
17+
<item name="encryption_update_key_command" xsi:type="object">Magento\EncryptionKey\Console\Command\UpdateEncryptionKeyCommand</item>
18+
</argument>
19+
</arguments>
20+
</type>
1421
</config>

app/code/Magento/ImportExport/Controller/Adminhtml/Export/File/Download.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use Magento\Framework\Filesystem;
1717
use Magento\ImportExport\Model\LocalizedFileName;
1818
use Throwable;
19+
use Magento\Framework\Controller\Result\Redirect;
20+
use Magento\Framework\App\ResponseInterface;
1921

2022
/**
2123
* Controller that download file by name.
@@ -25,7 +27,7 @@ class Download extends ExportController implements HttpGetActionInterface
2527
/**
2628
* Url to this controller
2729
*/
28-
const URL = 'adminhtml/export_file/download/';
30+
public const URL = 'adminhtml/export_file/download/';
2931

3032
/**
3133
* @var FileFactory
@@ -64,13 +66,24 @@ public function __construct(
6466
/**
6567
* Controller basic method implementation.
6668
*
67-
* @return \Magento\Framework\Controller\Result\Redirect | \Magento\Framework\App\ResponseInterface
69+
* @return Redirect|ResponseInterface
6870
*/
6971
public function execute()
7072
{
7173
$resultRedirect = $this->resultRedirectFactory->create();
7274
$resultRedirect->setPath('adminhtml/export/index');
75+
7376
$fileName = $this->getRequest()->getParam('filename');
77+
78+
if (empty($fileName)) {
79+
$this->messageManager->addErrorMessage(__('Please provide valid export file name'));
80+
81+
return $resultRedirect;
82+
}
83+
84+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
85+
$fileName = basename($fileName);
86+
7487
$exportDirectory = $this->filesystem->getDirectoryRead(DirectoryList::VAR_IMPORT_EXPORT);
7588

7689
try {

app/code/Magento/ImportExport/Controller/Adminhtml/History/Download.php

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
7+
declare(strict_types=1);
8+
69
namespace Magento\ImportExport\Controller\Adminhtml\History;
710

811
use Magento\Framework\App\Action\HttpGetActionInterface;
912
use Magento\Framework\App\Filesystem\DirectoryList;
13+
use Magento\Framework\Controller\ResultInterface;
14+
use Magento\ImportExport\Helper\Report;
1015
use Magento\ImportExport\Model\Import;
16+
use Magento\Framework\Controller\Result\Redirect;
17+
use Magento\Framework\App\ResponseInterface;
1118

1219
/**
1320
* Download history controller
@@ -44,20 +51,27 @@ public function __construct(
4451
/**
4552
* Download backup action
4653
*
47-
* @return void|\Magento\Backend\App\Action
54+
* @return ResponseInterface|Redirect|ResultInterface
55+
* @throws \Exception
4856
*/
4957
public function execute()
5058
{
59+
$resultRedirect = $this->resultRedirectFactory->create();
60+
$resultRedirect->setPath('*/history');
61+
62+
$fileName = $this->getRequest()->getParam('filename');
63+
64+
if (empty($fileName)) {
65+
return $resultRedirect;
66+
}
67+
5168
// phpcs:ignore Magento2.Functions.DiscouragedFunction
52-
$fileName = basename($this->getRequest()->getParam('filename'));
69+
$fileName = basename($fileName);
5370

54-
/** @var \Magento\ImportExport\Helper\Report $reportHelper */
55-
$reportHelper = $this->_objectManager->get(\Magento\ImportExport\Helper\Report::class);
71+
/** @var Report $reportHelper */
72+
$reportHelper = $this->_objectManager->get(Report::class);
5673

5774
if (!$reportHelper->importFileExists($fileName)) {
58-
/** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
59-
$resultRedirect = $this->resultRedirectFactory->create();
60-
$resultRedirect->setPath('*/history');
6175
return $resultRedirect;
6276
}
6377

app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/History/DownloadTest.php

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
namespace Magento\ImportExport\Test\Unit\Controller\Adminhtml\History;
99

1010
use Magento\Backend\App\Action\Context;
11-
use Magento\Backend\Model\View\Result\Redirect;
1211
use Magento\Framework\App\Filesystem\DirectoryList;
1312
use Magento\Framework\App\Request\Http;
1413
use Magento\Framework\App\Response\Http\FileFactory;
1514
use Magento\Framework\App\ResponseInterface;
1615
use Magento\Framework\Controller\Result\Raw;
1716
use Magento\Framework\Controller\Result\RawFactory;
17+
use Magento\Framework\Controller\Result\Redirect;
1818
use Magento\Framework\Controller\Result\RedirectFactory;
1919
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
2020
use Magento\ImportExport\Controller\Adminhtml\History\Download;
@@ -181,8 +181,7 @@ public function executeDataProvider()
181181
{
182182
return [
183183
'Normal file name' => ['filename.csv', 'filename.csv'],
184-
'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd'],
185-
'Empty file name' => ['', ''],
184+
'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd']
186185
];
187186
}
188187

@@ -196,4 +195,27 @@ public function testExecuteFileNotFound()
196195
$this->resultRaw->expects($this->never())->method('setContents');
197196
$this->downloadController->execute();
198197
}
198+
199+
/**
200+
* Test execute() with return Redirect
201+
* @param string|null $requestFilename
202+
* @dataProvider executeWithRedirectDataProvider
203+
*/
204+
public function testExecuteWithRedirect(?string $requestFilename): void
205+
{
206+
$this->request->method('getParam')->with('filename')->willReturn($requestFilename);
207+
$this->resultRaw->expects($this->never())->method('setContents');
208+
$this->assertSame($this->redirect, $this->downloadController->execute());
209+
}
210+
211+
/**
212+
* @return array
213+
*/
214+
public function executeWithRedirectDataProvider(): array
215+
{
216+
return [
217+
'null file name' => [null],
218+
'empty file name' => [''],
219+
];
220+
}
199221
}

0 commit comments

Comments
 (0)