From aee74f9674a15a88d5e9fba7f5250c88518ab621 Mon Sep 17 00:00:00 2001 From: sergey Date: Tue, 7 Jun 2022 00:11:59 +0300 Subject: [PATCH] Admin user can call arbitrary Module class's constructor via Cart Price Rule #35135 Redefined the check of the using class --- .../Controller/Adminhtml/Promo/Catalog.php | 2 +- .../Adminhtml/Promo/Catalog/NewActionHtml.php | 30 +++---- .../Promo/Catalog/NewConditionHtml.php | 31 +++++--- .../Adminhtml/Promo/Catalog/NewHtml.php | 69 ++++++++++++++++ .../Magento/Rule/view/adminhtml/web/rules.js | 4 +- .../Controller/Adminhtml/Promo/Quote.php | 38 +++++---- .../Adminhtml/Promo/Quote/NewActionHtml.php | 42 +++++----- .../Promo/Quote/NewConditionHtml.php | 42 +++++----- .../Adminhtml/Promo/Quote/NewHtml.php | 79 +++++++++++++++++++ 9 files changed, 251 insertions(+), 86 deletions(-) create mode 100644 app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewHtml.php create mode 100644 app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewHtml.php diff --git a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog.php b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog.php index c0c4b5c6fa12c..58566b57bf5df 100644 --- a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog.php +++ b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog.php @@ -43,7 +43,7 @@ abstract class Catalog extends Action /** * Date filter instance * - * @var \Magento\Framework\Stdlib\DateTime\Filter\Date + * @var Date */ protected $_dateFilter; diff --git a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewActionHtml.php b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewActionHtml.php index 65a73b62fd705..91135d027e475 100644 --- a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewActionHtml.php +++ b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewActionHtml.php @@ -4,15 +4,17 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ -namespace Magento\CatalogRule\Controller\Adminhtml\Promo\Catalog; -use Magento\Rule\Model\Action\AbstractAction; +declare(strict_types=1); +namespace Magento\CatalogRule\Controller\Adminhtml\Promo\Catalog; /** * @SuppressWarnings(PHPMD.AllPurposeAction) */ -class NewActionHtml extends \Magento\CatalogRule\Controller\Adminhtml\Promo\Catalog +class NewActionHtml extends NewHtml { + protected string $typeChecked = 'Magento\Rule\Model\Action\AbstractAction'; + /** * Execute new action html. * @@ -24,21 +26,21 @@ public function execute() $typeArr = explode('|', str_replace('-', '/', $this->getRequest()->getParam('type', ''))); $type = $typeArr[0]; - $model = $this->_objectManager->create($type) - ->setId($id) - ->setType($type) - ->setRule($this->_objectManager->create(\Magento\CatalogRule\Model\Rule::class)) - ->setPrefix('actions'); + $model = $this->_objectManager->create($type); + if ($this->verifyClassName($model)) { + $model->setId($id) + ->setType($type) + ->setRule($this->_objectManager->create(\Magento\CatalogRule\Model\Rule::class)) + ->setPrefix('actions'); - if (!empty($typeArr[1])) { - $model->setAttribute($typeArr[1]); - } + if (!empty($typeArr[1])) { + $model->setAttribute($typeArr[1]); + } - if ($model instanceof AbstractAction) { $model->setJsFormObject($this->getRequest()->getParam('form')); $html = $model->asHtmlRecursive(); - } else { - $html = ''; + }else { + $html = $this->getErrorJson(); } $this->getResponse()->setBody($html); } diff --git a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewConditionHtml.php b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewConditionHtml.php index 2aaaddf1e2b34..1849ad6f53a12 100644 --- a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewConditionHtml.php +++ b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewConditionHtml.php @@ -4,15 +4,18 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +declare(strict_types=1); namespace Magento\CatalogRule\Controller\Adminhtml\Promo\Catalog; +use Magento\CatalogRule\Model\Rule; use Magento\Framework\App\Action\HttpGetActionInterface; use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface; -use Magento\Rule\Model\Condition\AbstractCondition; -use Magento\CatalogRule\Controller\Adminhtml\Promo\Catalog as CatalogAction; -class NewConditionHtml extends CatalogAction implements HttpPostActionInterface, HttpGetActionInterface +class NewConditionHtml extends NewHtml implements HttpPostActionInterface, HttpGetActionInterface { + protected string $typeChecked = 'Magento\Rule\Model\Condition\AbstractCondition'; + /** * Execute new condition html. * @@ -25,23 +28,25 @@ public function execute() $typeArr = explode('|', str_replace('-', '/', $this->getRequest()->getParam('type', ''))); $type = $typeArr[0]; - $model = $this->_objectManager->create($type) - ->setId($id) - ->setType($type) - ->setRule($this->_objectManager->create(\Magento\CatalogRule\Model\Rule::class)) - ->setPrefix('conditions'); + $model = $this->_objectManager->create($type); - if (!empty($typeArr[1])) { - $model->setAttribute($typeArr[1]); - } + if ($this->verifyClassName($model)) { + $model->setId($id) + ->setType($type) + ->setRule($this->_objectManager->create(Rule::class)) + ->setPrefix('conditions'); + + if (!empty($typeArr[1])) { + $model->setAttribute($typeArr[1]); + } - if ($model instanceof AbstractCondition) { $model->setJsFormObject($this->getRequest()->getParam('form')); $model->setFormName($formName); $html = $model->asHtmlRecursive(); } else { - $html = ''; + $html = $this->getErrorJson(); } + $this->getResponse()->setBody($html); } } diff --git a/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewHtml.php b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewHtml.php new file mode 100644 index 0000000000000..7f80e0967492c --- /dev/null +++ b/app/code/Magento/CatalogRule/Controller/Adminhtml/Promo/Catalog/NewHtml.php @@ -0,0 +1,69 @@ +serializer = $serializer; + } + + /** + * Verify class instance + * + * @param mixed $verifyClass + * @return bool + */ + public function verifyClassName($verifyClass): bool + { + if ($verifyClass instanceof $this->typeChecked) { + return true; + } + + return false; + } + + /** + * Get Error json + * + * @return bool|string + */ + protected function getErrorJson() + { + return $this->serializer->serialize( + [ + 'error' => true, + 'message' => __('Selected type is not inherited from type %1', $this->typeChecked) + ] + ); + } +} \ No newline at end of file diff --git a/app/code/Magento/Rule/view/adminhtml/web/rules.js b/app/code/Magento/Rule/view/adminhtml/web/rules.js index 95175d272f9af..9e55048dc3997 100644 --- a/app/code/Magento/Rule/view/adminhtml/web/rules.js +++ b/app/code/Magento/Rule/view/adminhtml/web/rules.js @@ -347,9 +347,11 @@ define([ }, onComplete: this.onAddNewChildComplete.bind(this, new_elem), onSuccess: function (transport) { + let responseElement = ''; if (this._processSuccess(transport)) { - $(new_elem).update(transport.responseText); + responseElement = transport.responseText; } + $(new_elem).update(responseElement); }.bind(this), onFailure: this._processFailure.bind(this) }); diff --git a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote.php b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote.php index 55e1d319cc776..7116779c4c917 100644 --- a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote.php +++ b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote.php @@ -5,7 +5,15 @@ */ namespace Magento\SalesRule\Controller\Adminhtml\Promo; -abstract class Quote extends \Magento\Backend\App\Action +use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; +use Magento\Framework\App\Response\Http\FileFactory; +use Magento\Framework\Registry; +use Magento\Framework\Stdlib\DateTime\Filter\Date; +use Magento\SalesRule\Model\RegistryConstants; +use Magento\SalesRule\Model\Rule; + +abstract class Quote extends Action { /** * Authorization level of a basic admin session @@ -17,31 +25,31 @@ abstract class Quote extends \Magento\Backend\App\Action /** * Core registry * - * @var \Magento\Framework\Registry + * @var Registry */ protected $_coreRegistry = null; /** - * @var \Magento\Framework\App\Response\Http\FileFactory + * @var FileFactory */ protected $_fileFactory; /** - * @var \Magento\Framework\Stdlib\DateTime\Filter\Date + * @var Date */ protected $_dateFilter; /** - * @param \Magento\Backend\App\Action\Context $context - * @param \Magento\Framework\Registry $coreRegistry - * @param \Magento\Framework\App\Response\Http\FileFactory $fileFactory - * @param \Magento\Framework\Stdlib\DateTime\Filter\Date $dateFilter + * @param Context $context + * @param Registry $coreRegistry + * @param FileFactory $fileFactory + * @param Date $dateFilter */ public function __construct( - \Magento\Backend\App\Action\Context $context, - \Magento\Framework\Registry $coreRegistry, - \Magento\Framework\App\Response\Http\FileFactory $fileFactory, - \Magento\Framework\Stdlib\DateTime\Filter\Date $dateFilter + Context $context, + Registry $coreRegistry, + FileFactory $fileFactory, + Date $dateFilter ) { parent::__construct($context); $this->_coreRegistry = $coreRegistry; @@ -57,8 +65,8 @@ public function __construct( protected function _initRule() { $this->_coreRegistry->register( - \Magento\SalesRule\Model\RegistryConstants::CURRENT_SALES_RULE, - $this->_objectManager->create(\Magento\SalesRule\Model\Rule::class) + RegistryConstants::CURRENT_SALES_RULE, + $this->_objectManager->create(Rule::class) ); $id = (int)$this->getRequest()->getParam('id'); @@ -67,7 +75,7 @@ protected function _initRule() } if ($id) { - $this->_coreRegistry->registry(\Magento\SalesRule\Model\RegistryConstants::CURRENT_SALES_RULE)->load($id); + $this->_coreRegistry->registry(RegistryConstants::CURRENT_SALES_RULE)->load($id); } } diff --git a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewActionHtml.php b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewActionHtml.php index 47c9f41453b74..87c0c992283f8 100644 --- a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewActionHtml.php +++ b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewActionHtml.php @@ -3,18 +3,23 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +declare(strict_types=1); namespace Magento\SalesRule\Controller\Adminhtml\Promo\Quote; -use Magento\Framework\App\Action\HttpPostActionInterface; -use Magento\Rule\Model\Condition\AbstractCondition; -use Magento\SalesRule\Controller\Adminhtml\Promo\Quote; use Magento\SalesRule\Model\Rule; +use Magento\Rule\Model\Condition\AbstractCondition; /** * New action html action */ -class NewActionHtml extends Quote implements HttpPostActionInterface +class NewActionHtml extends NewHtml { + /** + * @var string + */ + protected string $typeChecked = 'Magento\Rule\Model\Condition\AbstractCondition'; + /** * New action html action * @@ -30,31 +35,24 @@ public function execute() ); $type = $typeArr[0]; - $model = $this->_objectManager->create( - $type - )->setId( - $id - )->setType( - $type - )->setRule( - $this->_objectManager->create(Rule::class) - )->setPrefix( - 'actions' - ); - if (!empty($typeArr[1])) { - $model->setAttribute($typeArr[1]); - } + $model = $this->_objectManager->create($type); + if ($this->verifyClassName($model)) { + $model->setId($id) + ->setType($type) + ->setRule($this->_objectManager->create(Rule::class)) + ->setPrefix('actions'); + if (!empty($typeArr[1])) { + $model->setAttribute($typeArr[1]); + } - if ($model instanceof AbstractCondition) { $model->setJsFormObject($formName); $model->setFormName($formName); $this->setJsFormObject($model); $html = $model->asHtmlRecursive(); } else { - $html = ''; + $html = $this->getErrorJson(); } - $this->getResponse() - ->setBody($html); + $this->getResponse()->setBody($html); } /** diff --git a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewConditionHtml.php b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewConditionHtml.php index 959e3cf192f6d..97429df722e65 100644 --- a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewConditionHtml.php +++ b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewConditionHtml.php @@ -3,17 +3,24 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +declare(strict_types=1); + namespace Magento\SalesRule\Controller\Adminhtml\Promo\Quote; -use Magento\Framework\App\Action\HttpPostActionInterface; +use Magento\SalesRule\Model\Rule; use Magento\Rule\Model\Condition\AbstractCondition; -use Magento\SalesRule\Controller\Adminhtml\Promo\Quote; /** * Controller class NewConditionHtml. Returns condition html */ -class NewConditionHtml extends Quote implements HttpPostActionInterface +class NewConditionHtml extends NewHtml { + /** + * @var string + */ + protected string $typeChecked = 'Magento\Rule\Model\Condition\AbstractCondition'; + /** * New condition html action * @@ -29,29 +36,24 @@ public function execute() ); $type = $typeArr[0]; - $model = $this->_objectManager->create( - $type - )->setId( - $id - )->setType( - $type - )->setRule( - $this->_objectManager->create(\Magento\SalesRule\Model\Rule::class) - )->setPrefix( - 'conditions' - ); - if (!empty($typeArr[1])) { - $model->setAttribute($typeArr[1]); - } + $model = $this->_objectManager->create($type); + if ($this->verifyClassName($model)) { + $model->setId($id) + ->setType($type) + ->setRule($this->_objectManager->create(Rule::class)) + ->setPrefix('conditions'); + if (!empty($typeArr[1])) { + $model->setAttribute($typeArr[1]); + } - if ($model instanceof AbstractCondition) { $model->setJsFormObject($this->getRequest()->getParam('form')); $model->setFormName($formName); $this->setJsFormObject($model); $html = $model->asHtmlRecursive(); - } else { - $html = ''; + }else { + $html = $this->getErrorJson(); } + $this->getResponse()->setBody($html); } diff --git a/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewHtml.php b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewHtml.php new file mode 100644 index 0000000000000..6f695af8c495d --- /dev/null +++ b/app/code/Magento/SalesRule/Controller/Adminhtml/Promo/Quote/NewHtml.php @@ -0,0 +1,79 @@ +serializer = $serializer; + } + + /** + * Verify class instance + * + * @param mixed $verifyClass + * @return bool + */ + public function verifyClassName($verifyClass): bool + { + if ($verifyClass instanceof $this->typeChecked) { + return true; + } + + return false; + } + + /** + * Get Error json + * + * @return bool|string + */ + protected function getErrorJson() + { + return $this->serializer->serialize( + [ + 'error' => true, + 'message' => __('Selected type is not inherited from type %1', $this->typeChecked) + ] + ); + } +} \ No newline at end of file