Skip to content

Commit

Permalink
Security changes from upstream 2.4.7-p2 (#101)
Browse files Browse the repository at this point in the history
  • Loading branch information
rhoerr authored Aug 20, 2024
1 parent 1089058 commit acd6ffc
Show file tree
Hide file tree
Showing 52 changed files with 469 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class ValidatorInfo extends Validator
* @var IoFile
*/
private $ioFile;

/**
* @var NotProtectedExtension
*/
Expand Down Expand Up @@ -147,12 +148,14 @@ private function validatePath(array $optionValuePath): bool
{
foreach ([$optionValuePath['quote_path'], $optionValuePath['order_path']] as $path) {
$pathInfo = $this->ioFile->getPathInfo($path);
if (isset($pathInfo['extension'])) {
if (!$this->fileValidator->isValid($pathInfo['extension'])) {
return false;
}

if (isset($pathInfo['extension'])
&& (empty($pathInfo['extension']) || !$this->fileValidator->isValid($pathInfo['extension']))
) {
return false;
}
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
<arguments>
<argument name="fileName" defaultValue="magento-logo" type="string"/>
</arguments>
<seeElement selector="{{StorefrontProductMediaSection.productImageActive(fileName)}}" stepKey="seeActiveImageDefault"/>
<waitForElementVisible selector="{{StorefrontProductMediaSection.productImageActive(fileName)}}" stepKey="seeActiveImageDefault"/>
</actionGroup>
</actionGroups>
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@
<comment userInput="Admin creates category." stepKey="adminCreatesCategoryComment" before="navigateToCategoryPage"/>
<actionGroup ref="AdminOpenCategoryPageActionGroup" stepKey="navigateToCategoryPage"/>
<!--Create category under Default Category-->
<click selector="{{AdminCategorySidebarTreeSection.expandCategoryByName('Default Category')}}" stepKey="clickDefaultCategory"/>
<click selector="{{AdminCategorySidebarTreeSection.categoryInTree(_defaultCategory.name)}}" stepKey="clickDefaultCategory"/>
<actionGroup ref="CheckCategoryNameIsRequiredFieldActionGroup" stepKey="checkCategoryNameIsRequired"/>
<actionGroup ref="CreateCategoryActionGroup" stepKey="createCategory">
<argument name="categoryEntity" value="_defaultCategory"/>
Expand Down
6 changes: 4 additions & 2 deletions app/code/Magento/Config/Model/Config/Backend/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,10 @@ protected function _getAllowedExtensions()
*/
private function setValueAfterValidation(string $value): void
{
// avoid intercepting value
if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)) {
if (preg_match('/[^a-z0-9_\/\\-\\.]+/i', $value)
// phpcs:ignore Magento2.Functions.DiscouragedFunction
|| !$this->_mediaDirectory->isFile($this->_getUploadDir() . DIRECTORY_SEPARATOR . basename($value))
) {
throw new LocalizedException(__('Invalid file name'));
}

Expand Down
4 changes: 3 additions & 1 deletion app/code/Magento/Customer/Model/Plugin/UpdateCustomer.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ public function beforeSave(
$customerId === $customerSessionId
) {
$customer = $this->getUpdatedCustomer($customerRepository->getById($customerId), $customer);
} elseif ($userType === UserContextInterface::USER_TYPE_ADMIN && $customerId) {
} elseif ($customerId && in_array($userType, [UserContextInterface::USER_TYPE_ADMIN,
UserContextInterface::USER_TYPE_INTEGRATION], true)
) {
$customer = $this->getUpdatedCustomer($customerRepository->getById($customerId), $customer);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\EncryptionKey\Console\Command;

use Magento\Framework\App\DeploymentConfig\Writer;
use Magento\Framework\Config\ConfigOptionsListConstants;
use Magento\Framework\Config\Data\ConfigData;
use Magento\Framework\Config\File\ConfigFilePool;
use Magento\Framework\Exception\FileSystemException;
use Magento\Framework\Math\Random;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Magento\Framework\App\CacheInterface;
use Magento\Framework\Encryption\EncryptorInterface;

class UpdateEncryptionKeyCommand extends Command
{
/**
* @var EncryptorInterface
*/
private EncryptorInterface $encryptor;

/**
* @var CacheInterface
*/
private CacheInterface $cache;

/**
* Configuration writer
*
* @var Writer
*/
private Writer $writer;

/**
* Random string generator
*
* @var Random
*/
private Random $random;

/**
* @param EncryptorInterface $encryptor
* @param CacheInterface $cache
* @param Writer $writer
* @param Random $random
*/
public function __construct(EncryptorInterface $encryptor, CacheInterface $cache, Writer $writer, Random $random)
{
$this->encryptor = $encryptor;
$this->cache = $cache;
$this->writer = $writer;
$this->random = $random;

parent::__construct();
}

/**
* @inheritDoc
*/
protected function configure()
{
$this->setName('encryption:key:change');
$this->setDescription('Change the encryption key inside the env.php file.');
$this->addOption(
'key',
'k',
InputOption::VALUE_OPTIONAL,
'Key has to be a 32 characters long string. If not provided, a random key will be generated.'
);

parent::configure();
}

/**
* @inheritDoc
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
try {
$key = $input->getOption('key');

if (!empty($key)) {
$this->encryptor->validateKey($key);
}

$this->updateEncryptionKey($key);
$this->cache->clean();

$output->writeln('<info>Encryption key has been updated successfully.</info>');

return Command::SUCCESS;
} catch (\Exception $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
return Command::FAILURE;
}
}

/**
* Update encryption key
*
* @param string|null $key
* @return void
* @throws FileSystemException
*/
private function updateEncryptionKey(string $key = null): void
{
// prepare new key, encryptor and new configuration segment
if (!$this->writer->checkIfWritable()) {
throw new FileSystemException(__('Deployment configuration file is not writable.'));
}

if (null === $key) {
$key = ConfigOptionsListConstants::STORE_KEY_ENCODED_RANDOM_STRING_PREFIX .
$this->random->getRandomBytes(ConfigOptionsListConstants::STORE_KEY_RANDOM_STRING_SIZE);
}

$this->encryptor->setNewKey($key);

$encryptSegment = new ConfigData(ConfigFilePool::APP_ENV);
$encryptSegment->set(ConfigOptionsListConstants::CONFIG_PATH_CRYPT_KEY, $this->encryptor->exportKeys());

$configData = [$encryptSegment->getFileKey() => $encryptSegment->getData()];

$this->writer->saveConfig($configData);
}
}
7 changes: 7 additions & 0 deletions app/code/Magento/EncryptionKey/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,11 @@
<argument name="structure" xsi:type="object">Magento\Config\Model\Config\Structure\Proxy</argument>
</arguments>
</type>
<type name="Magento\Framework\Console\CommandList">
<arguments>
<argument name="commands" xsi:type="array">
<item name="encryption_update_key_command" xsi:type="object">Magento\EncryptionKey\Console\Command\UpdateEncryptionKeyCommand</item>
</argument>
</arguments>
</type>
</config>
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
use Magento\Framework\Filesystem;
use Magento\ImportExport\Model\LocalizedFileName;
use Throwable;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\App\ResponseInterface;

/**
* Controller that download file by name.
Expand All @@ -25,7 +27,7 @@ class Download extends ExportController implements HttpGetActionInterface
/**
* Url to this controller
*/
const URL = 'adminhtml/export_file/download/';
public const URL = 'adminhtml/export_file/download/';

/**
* @var FileFactory
Expand Down Expand Up @@ -64,13 +66,24 @@ public function __construct(
/**
* Controller basic method implementation.
*
* @return \Magento\Framework\Controller\Result\Redirect | \Magento\Framework\App\ResponseInterface
* @return Redirect|ResponseInterface
*/
public function execute()
{
$resultRedirect = $this->resultRedirectFactory->create();
$resultRedirect->setPath('adminhtml/export/index');

$fileName = $this->getRequest()->getParam('filename');

if (empty($fileName)) {
$this->messageManager->addErrorMessage(__('Please provide valid export file name'));

return $resultRedirect;
}

// phpcs:ignore Magento2.Functions.DiscouragedFunction
$fileName = basename($fileName);

$exportDirectory = $this->filesystem->getDirectoryRead(DirectoryList::VAR_IMPORT_EXPORT);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\ImportExport\Controller\Adminhtml\History;

use Magento\Framework\App\Action\HttpGetActionInterface;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\Controller\ResultInterface;
use Magento\ImportExport\Helper\Report;
use Magento\ImportExport\Model\Import;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\App\ResponseInterface;

/**
* Download history controller
Expand Down Expand Up @@ -44,20 +51,27 @@ public function __construct(
/**
* Download backup action
*
* @return void|\Magento\Backend\App\Action
* @return ResponseInterface|Redirect|ResultInterface
* @throws \Exception
*/
public function execute()
{
$resultRedirect = $this->resultRedirectFactory->create();
$resultRedirect->setPath('*/history');

$fileName = $this->getRequest()->getParam('filename');

if (empty($fileName)) {
return $resultRedirect;
}

// phpcs:ignore Magento2.Functions.DiscouragedFunction
$fileName = basename($this->getRequest()->getParam('filename'));
$fileName = basename($fileName);

/** @var \Magento\ImportExport\Helper\Report $reportHelper */
$reportHelper = $this->_objectManager->get(\Magento\ImportExport\Helper\Report::class);
/** @var Report $reportHelper */
$reportHelper = $this->_objectManager->get(Report::class);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
namespace Magento\ImportExport\Test\Unit\Controller\Adminhtml\History;

use Magento\Backend\App\Action\Context;
use Magento\Backend\Model\View\Result\Redirect;
use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\Request\Http;
use Magento\Framework\App\Response\Http\FileFactory;
use Magento\Framework\App\ResponseInterface;
use Magento\Framework\Controller\Result\Raw;
use Magento\Framework\Controller\Result\RawFactory;
use Magento\Framework\Controller\Result\Redirect;
use Magento\Framework\Controller\Result\RedirectFactory;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
use Magento\ImportExport\Controller\Adminhtml\History\Download;
Expand Down Expand Up @@ -181,8 +181,7 @@ public function executeDataProvider()
{
return [
'Normal file name' => ['filename.csv', 'filename.csv'],
'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd'],
'Empty file name' => ['', ''],
'Relative file name' => ['../../../../../../../../etc/passwd', 'passwd']
];
}

Expand All @@ -196,4 +195,27 @@ public function testExecuteFileNotFound()
$this->resultRaw->expects($this->never())->method('setContents');
$this->downloadController->execute();
}

/**
* Test execute() with return Redirect
* @param string|null $requestFilename
* @dataProvider executeWithRedirectDataProvider
*/
public function testExecuteWithRedirect(?string $requestFilename): void
{
$this->request->method('getParam')->with('filename')->willReturn($requestFilename);
$this->resultRaw->expects($this->never())->method('setContents');
$this->assertSame($this->redirect, $this->downloadController->execute());
}

/**
* @return array
*/
public function executeWithRedirectDataProvider(): array
{
return [
'null file name' => [null],
'empty file name' => [''],
];
}
}
Loading

0 comments on commit acd6ffc

Please sign in to comment.