From e9ead27c08ad8b8982cca73744498e73f6f6440d Mon Sep 17 00:00:00 2001 From: Yaroslav Voronoy Date: Wed, 6 Jan 2016 10:11:23 +0200 Subject: [PATCH 01/73] MDVA-57: The Security Bundle Jan 2016 Merchant Beta - MAGETWO-46920: SQLi Vulnerability --- .../CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php b/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php index 39507f7f2d009..dea947e45769f 100644 --- a/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php +++ b/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php @@ -119,10 +119,10 @@ private function processQueryWithField(FilterInterface $filter, $isNegation, $qu $value = sprintf( '%s IN (%s)', ($isNegation ? 'NOT' : ''), - implode(',', $filter->getValue()) + $this->connection->quote(implode(',', $filter->getValue())) ); } else { - $value = ($isNegation ? '!' : '') . '= ' . $filter->getValue(); + $value = ($isNegation ? '!' : '') . '= ' . $this->connection->quote($filter->getValue()); } $filterQuery = sprintf( '%1$s.value %2$s', From f64d50e60150555c02133a4ce0a9651814f7c450 Mon Sep 17 00:00:00 2001 From: Yaroslav Voronoy Date: Wed, 6 Jan 2016 10:15:56 +0200 Subject: [PATCH 02/73] MDVA-57: The Security Bundle Jan 2016 Merchant Beta - MAGETWO-46920: SQLi Vulnerability --- .../CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php | 4 ++-- .../Magento/Framework/Search/AbstractKeyValuePair.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php b/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php index dea947e45769f..3e291d68e074a 100644 --- a/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php +++ b/app/code/Magento/CatalogSearch/Model/Adapter/Mysql/Filter/Preprocessor.php @@ -104,7 +104,7 @@ private function processQueryWithField(FilterInterface $filter, $isNegation, $qu ); return $filterQuery; } elseif ($filter->getField() === 'category_ids') { - return 'category_ids_index.category_id = ' . $filter->getValue(); + return 'category_ids_index.category_id = ' . (int) $filter->getValue(); } elseif ($attribute->isStatic()) { $alias = $this->tableMapper->getMappingAlias($filter); $filterQuery = str_replace( @@ -119,7 +119,7 @@ private function processQueryWithField(FilterInterface $filter, $isNegation, $qu $value = sprintf( '%s IN (%s)', ($isNegation ? 'NOT' : ''), - $this->connection->quote(implode(',', $filter->getValue())) + implode(',', array_map([$this->connection, 'quote'], $filter->getValue())) ); } else { $value = ($isNegation ? '!' : '') . '= ' . $this->connection->quote($filter->getValue()); diff --git a/lib/internal/Magento/Framework/Search/AbstractKeyValuePair.php b/lib/internal/Magento/Framework/Search/AbstractKeyValuePair.php index 392ec1159fd9c..9117e94ed43f3 100644 --- a/lib/internal/Magento/Framework/Search/AbstractKeyValuePair.php +++ b/lib/internal/Magento/Framework/Search/AbstractKeyValuePair.php @@ -46,7 +46,7 @@ public function getName() /** * Get field values * - * @return mixed + * @return mixed Return data in raw-formt. Must be escaped for using in sq * @codeCoverageIgnore */ public function getValue() From 87b76bd8907576dec4ee6b20dfa558429903db8e Mon Sep 17 00:00:00 2001 From: Yaroslav Voronoy Date: Wed, 6 Jan 2016 10:28:37 +0200 Subject: [PATCH 03/73] MDVA-57: The Security Bundle Jan 2016 Merchant Beta - MAGETWO-46920: SQLi Vulnerability --- .../Adapter/Mysql/Filter/PreprocessorTest.php | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogSearch/Test/Unit/Model/Adapter/Mysql/Filter/PreprocessorTest.php b/app/code/Magento/CatalogSearch/Test/Unit/Model/Adapter/Mysql/Filter/PreprocessorTest.php index 2e5241e8e91f0..bc956a927f476 100644 --- a/app/code/Magento/CatalogSearch/Test/Unit/Model/Adapter/Mysql/Filter/PreprocessorTest.php +++ b/app/code/Magento/CatalogSearch/Test/Unit/Model/Adapter/Mysql/Filter/PreprocessorTest.php @@ -103,7 +103,7 @@ protected function setUp() ->getMock(); $this->connection = $this->getMockBuilder('\Magento\Framework\DB\Adapter\AdapterInterface') ->disableOriginalConstructor() - ->setMethods(['select', 'getIfNullSql']) + ->setMethods(['select', 'getIfNullSql', 'quote']) ->getMockForAbstractClass(); $this->select = $this->getMockBuilder('\Magento\Framework\DB\Select') ->disableOriginalConstructor() @@ -171,9 +171,25 @@ public function testProcessPrice() $this->assertSame($expectedResult, $this->removeWhitespaces($actualResult)); } - public function testProcessCategoryIds() + /** + * @return array + */ + public function processCategoryIdsDataProvider() + { + return [ + ['5', 'category_ids_index.category_id = 5'], + [3, 'category_ids_index.category_id = 3'], + ["' and 1 = 0", 'category_ids_index.category_id = 0'], + ]; + } + + /** + * @param string|int $categoryId + * @param string $expectedResult + * @dataProvider processCategoryIdsDataProvider + */ + public function testProcessCategoryIds($categoryId, $expectedResult) { - $expectedResult = 'category_ids_index.category_id = FilterValue'; $scopeId = 0; $isNegation = false; $query = 'SELECT category_ids FROM catalog_product_entity'; @@ -185,7 +201,7 @@ public function testProcessCategoryIds() $this->filter->expects($this->once()) ->method('getValue') - ->will($this->returnValue('FilterValue')); + ->will($this->returnValue($categoryId)); $this->config->expects($this->exactly(1)) ->method('getAttribute') @@ -222,6 +238,7 @@ public function testProcessStaticAttribute() ->method('getBackendTable') ->will($this->returnValue('backend_table')); + $this->connection->expects($this->atLeastOnce())->method('quote')->willReturnArgument(0); $actualResult = $this->target->process($this->filter, $isNegation, $query); $this->assertSame($expectedResult, $this->removeWhitespaces($actualResult)); } From 9f5dce3646b25f55f5e1db98dd6f2c12e5fe00e3 Mon Sep 17 00:00:00 2001 From: Leonid Poluyanov Date: Thu, 3 Dec 2015 15:24:33 +0200 Subject: [PATCH 04/73] MAGETWO-46016: Delete / Edit Reviews --- .../Magento/Review/Controller/Product/Post.php | 14 +++++++++++++- .../Test/Unit/Controller/Product/PostTest.php | 12 +++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Review/Controller/Product/Post.php b/app/code/Magento/Review/Controller/Product/Post.php index a68f0d18c3bc5..27d5eb9620852 100644 --- a/app/code/Magento/Review/Controller/Product/Post.php +++ b/app/code/Magento/Review/Controller/Product/Post.php @@ -37,7 +37,7 @@ public function execute() $data = $this->getRequest()->getPostValue(); $rating = $this->getRequest()->getParam('ratings', []); } - + $data = $this->filterData($data); if (($product = $this->initProduct()) && !empty($data)) { /** @var \Magento\Review\Model\Review $review */ $review = $this->reviewFactory->create()->setData($data); @@ -82,4 +82,16 @@ public function execute() $resultRedirect->setUrl($redirectUrl ?: $this->_redirect->getRedirectUrl()); return $resultRedirect; } + + /** + * @param array $data + * @return array + */ + protected function filterData(array $data) + { + if (!empty($data['review_id'])) { + unset($data['review_id']); + } + return $data; + } } diff --git a/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php b/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php index 9cedb87e15afe..522db881e12ca 100644 --- a/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php +++ b/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php @@ -219,7 +219,13 @@ public function setUp() */ public function testExecute() { - $ratingsData = ['ratings' => [1 => 1]]; + $reviewData = [ + 'ratings' => [1 => 1] + ]; + $reviewDataWithId = [ + 'ratings' => [1 => 1], + 'review_id' => 2 + ]; $productId = 1; $customerId = 1; $storeId = 1; @@ -230,7 +236,7 @@ public function testExecute() ->willReturn(true); $this->reviewSession->expects($this->any())->method('getFormData') ->with(true) - ->willReturn($ratingsData); + ->willReturn($reviewDataWithId); $this->request->expects($this->at(0))->method('getParam') ->with('category', false) ->willReturn(false); @@ -260,7 +266,7 @@ public function testExecute() ->with('product', $product) ->willReturnSelf(); $this->review->expects($this->once())->method('setData') - ->with($ratingsData) + ->with($reviewData) ->willReturnSelf(); $this->review->expects($this->once())->method('validate') ->willReturn(true); From 3961e03a7bb56fa0f664ce8be303f89300490e95 Mon Sep 17 00:00:00 2001 From: Leonid Poluyanov Date: Tue, 8 Dec 2015 19:16:22 +0200 Subject: [PATCH 05/73] MAGETWO-46016: Delete / Edit Reviews --- .../Magento/Review/Controller/Product/Post.php | 14 +------------- .../Test/Unit/Controller/Product/PostTest.php | 8 +++----- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/app/code/Magento/Review/Controller/Product/Post.php b/app/code/Magento/Review/Controller/Product/Post.php index 27d5eb9620852..e20872b41dcd6 100644 --- a/app/code/Magento/Review/Controller/Product/Post.php +++ b/app/code/Magento/Review/Controller/Product/Post.php @@ -37,10 +37,10 @@ public function execute() $data = $this->getRequest()->getPostValue(); $rating = $this->getRequest()->getParam('ratings', []); } - $data = $this->filterData($data); if (($product = $this->initProduct()) && !empty($data)) { /** @var \Magento\Review\Model\Review $review */ $review = $this->reviewFactory->create()->setData($data); + $review->unsetData('review_id'); $validate = $review->validate(); if ($validate === true) { @@ -82,16 +82,4 @@ public function execute() $resultRedirect->setUrl($redirectUrl ?: $this->_redirect->getRedirectUrl()); return $resultRedirect; } - - /** - * @param array $data - * @return array - */ - protected function filterData(array $data) - { - if (!empty($data['review_id'])) { - unset($data['review_id']); - } - return $data; - } } diff --git a/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php b/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php index 522db881e12ca..38f1a27a4d430 100644 --- a/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php +++ b/app/code/Magento/Review/Test/Unit/Controller/Product/PostTest.php @@ -127,7 +127,7 @@ public function setUp() '\Magento\Review\Model\Review', [ 'setData', 'validate', 'setEntityId', 'getEntityIdByCode', 'setEntityPkValue', 'setStatusId', - 'setCustomerId', 'setStoreId', 'setStores', 'save', 'getId', 'aggregate' + 'setCustomerId', 'setStoreId', 'setStores', 'save', 'getId', 'aggregate', 'unsetData' ], [], '', @@ -220,9 +220,6 @@ public function setUp() public function testExecute() { $reviewData = [ - 'ratings' => [1 => 1] - ]; - $reviewDataWithId = [ 'ratings' => [1 => 1], 'review_id' => 2 ]; @@ -236,7 +233,7 @@ public function testExecute() ->willReturn(true); $this->reviewSession->expects($this->any())->method('getFormData') ->with(true) - ->willReturn($reviewDataWithId); + ->willReturn($reviewData); $this->request->expects($this->at(0))->method('getParam') ->with('category', false) ->willReturn(false); @@ -276,6 +273,7 @@ public function testExecute() $this->review->expects($this->once())->method('setEntityId') ->with(1) ->willReturnSelf(); + $this->review->expects($this->once())->method('unsetData')->with('review_id'); $product->expects($this->exactly(2)) ->method('getId') ->willReturn($productId); From ba4647e74bdcbdb7facf3f2e7daa3093e8e3f80a Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Mon, 30 Nov 2015 19:03:06 +0200 Subject: [PATCH 06/73] MAGETWO-45954: Stored XSS through custom options Conflicts: app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml --- .../product/composite/fieldset/options/type/file.phtml | 6 +++--- .../frontend/templates/product/view/options/type/file.phtml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml index 28bc783f49dfd..6a24526ae4e28 100644 --- a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml +++ b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml @@ -68,9 +68,9 @@ require(['prototype'], function(){
- getTitle(); ?> - - + escapeHtml($_fileInfo->getTitle()); ?> + +   getIsRequire()): ?> diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml index 9e8165e3da4c0..69e7382ae0e06 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml @@ -23,9 +23,9 @@
- getTitle(); ?> - - + escapeHtml($_fileInfo->getTitle()); ?> + + getIsRequire()): ?> From 738851b85e4409a006a32ea5b5bfb88e7abbea3a Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Thu, 3 Dec 2015 13:38:33 +0200 Subject: [PATCH 07/73] MAGETWO-45954: Stored XSS through custom options - fix CS Conflicts: app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml --- .../composite/fieldset/options/type/file.phtml | 6 +++--- .../product/view/options/type/file.phtml | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml index 6a24526ae4e28..6823e7d7c6047 100644 --- a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml +++ b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/composite/fieldset/options/type/file.phtml @@ -68,7 +68,7 @@ require(['prototype'], function(){
- escapeHtml($_fileInfo->getTitle()); ?> + escapeHtml($_fileInfo->getTitle()); ?>   @@ -79,8 +79,8 @@ require(['prototype'], function(){
> - /> - + /> + getFileExtension()): ?>
diff --git a/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml b/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml index 69e7382ae0e06..0a100f27b9aed 100644 --- a/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml +++ b/app/code/Magento/Catalog/view/frontend/templates/product/view/options/type/file.phtml @@ -16,15 +16,15 @@ getIsRequire()) ? ' required' : ''; ?> -
-