Merge pull request magento#4924 from magento-arcticfoxes/MC-13777

[arcticfoxes] MC-13777: [Backport for 2.3.x] [PSIRT-9529] XPAth Injection vulnerability on front end of site
Hypernova-Oy · Oct 25, 2019 · cb12aa6 · cb12aa6
2 parents 3e1bd1e + da52d60
commit cb12aa6
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 5 deletions.
diff --git a/app/code/Magento/PageCache/Controller/Block.php b/app/code/Magento/PageCache/Controller/Block.php
@@ -11,6 +11,9 @@
 use Magento\Framework\Serialize\Serializer\Json;
 use Magento\Framework\View\Layout\LayoutCacheKeyInterface;
 
+/**
+ * Page cache block controller abstract class
+ */
 abstract class Block extends \Magento\Framework\App\Action\Action
 {
     /**
@@ -72,13 +75,12 @@ public function __construct(
     protected function _getBlocks()
     {
         $blocks = $this->getRequest()->getParam('blocks', '');
-        $handles = $this->getRequest()->getParam('handles', '');
+        $handles = $this->getHandles();
 
         if (!$handles || !$blocks) {
             return [];
         }
         $blocks = $this->jsonSerializer->unserialize($blocks);
-        $handles = $this->base64jsonSerializer->unserialize($handles);
 
         $layout = $this->_view->getLayout();
         $this->layoutCacheKey->addCacheKeys($this->layoutCacheKeyName);
@@ -95,4 +97,22 @@ protected function _getBlocks()
 
         return $data;
     }
+
+    /**
+     * Get handles
+     *
+     * @return array
+     */
+    private function getHandles(): array
+    {
+        $handles = $this->getRequest()->getParam('handles', '');
+        $handles = !$handles ? [] : $this->base64jsonSerializer->unserialize($handles);
+        $validHandles = [];
+        foreach ($handles as $handle) {
+            if (!preg_match('/[@\'\*\.\\\"]/i', $handle)) {
+                $validHandles[] = $handle;
+            }
+        }
+        return $validHandles;
+    }
 }
diff --git a/app/code/Magento/PageCache/Controller/Block/Render.php b/app/code/Magento/PageCache/Controller/Block/Render.php
@@ -6,7 +6,14 @@
  */
 namespace Magento\PageCache\Controller\Block;
 
-class Render extends \Magento\PageCache\Controller\Block
+use Magento\Framework\App\Action\HttpGetActionInterface;
+
+/**
+ * Page cache render controller
+ *
+ * @deprecated
+ */
+class Render extends \Magento\PageCache\Controller\Block implements HttpGetActionInterface
 {
     /**
      * Returns block content depends on ajax request

diff --git a/app/code/Magento/PageCache/Test/Unit/Controller/Block/RenderTest.php b/app/code/Magento/PageCache/Test/Unit/Controller/Block/RenderTest.php
@@ -134,7 +134,7 @@ public function testExecuteNoParams()
     public function testExecute()
     {
         $blocks = ['block1', 'block2'];
-        $handles = ['handle1', 'handle2'];
+        $handles = ['handle1', 'handle2', "'handle'", '@hanle', '"hanle', '*hanle', '.hanle'];
         $originalRequest = '{"route":"route","controller":"controller","action":"action","uri":"uri"}';
         $expectedData = ['block1' => 'data1', 'block2' => 'data2'];
 
@@ -177,7 +177,7 @@ public function testExecute()
             ->method('getParam')
             ->with($this->equalTo('handles'), $this->equalTo(''))
             ->will($this->returnValue(base64_encode(json_encode($handles))));
-        $this->viewMock->expects($this->once())->method('loadLayout')->with($this->equalTo($handles));
+        $this->viewMock->expects($this->once())->method('loadLayout')->with($this->equalTo(['handle1', 'handle2']));
         $this->viewMock->expects($this->any())->method('getLayout')->will($this->returnValue($this->layoutMock));
         $this->layoutMock->expects($this->never())
             ->method('getUpdate');