Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security changes from upstream 2.4.7-p2 #101

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading