MAGETWO-97961: Fixed incorrect email template rendering

Author: viktorpetryk

lib/internal/Magento/Framework/Filter/Template.php

@@ -9,6 +9,9 @@
  */
 namespace Magento\Framework\Filter;
 
+use Magento\Framework\Model\AbstractExtensibleModel;
+use Magento\Framework\Model\AbstractModel;
+
 class Template implements \Zend_Filter_Interface
 {
     /**
@@ -53,6 +56,19 @@ class Template implements \Zend_Filter_Interface
      */
     protected $string;
 
+    /**
+     * @var string[]
+     */
+    private $restrictedMethods = [
+        'addafterfiltercallback',
+        'getresourcecollection',
+        'load',
+        'save',
+        'getcollection',
+        'getresource',
+        'getconfig',
+    ];
+
     /**
      * @param \Magento\Framework\Stdlib\StringUtils $string
      * @param array $variables
@@ -298,6 +314,46 @@ protected function getParameters($value)
         return $params;
     }
 
+    /**
+     * Validate method call initiated in a template.
+     *
+     * Deny calls for methods that may disrupt template processing.
+     *
+     * @param object $object
+     * @param string $method
+     * @return void
+     * @throws \InvalidArgumentException
+     */
+    private function validateVariableMethodCall($object, string $method)
+    {
+        if ($object === $this) {
+            if (in_array(mb_strtolower($method), $this->restrictedMethods)) {
+                throw new \InvalidArgumentException("Method $method cannot be called from template.");
+            }
+        }
+    }
+
+    /**
+     * Check allowed methods for data objects.
+     *
+     * Deny calls for methods that may disrupt template processing.
+     *
+     * @param object $object
+     * @param string $method
+     * @return bool
+     * @throws \InvalidArgumentException
+     */
+    private function isAllowedDataObjectMethod($object, string $method)
+    {
+        if ($object instanceof AbstractExtensibleModel || $object instanceof AbstractModel) {
+            if (in_array(mb_strtolower($method), $this->restrictedMethods)) {
+                throw new \InvalidArgumentException("Method $method cannot be called from template.");
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Return variable value for var construction
      *
@@ -322,7 +378,7 @@ protected function getVariable($value, $default = '{no_value_defined}')
                     isset($stackVars[$i - 1]['variable'])
                     && $stackVars[$i - 1]['variable'] instanceof \Magento\Framework\DataObject
             ) {
-                // If object calling methods or getting properties
+                // If data object calling methods or getting properties
                 if ($stackVars[$i]['type'] == 'property') {
                     $caller = 'get' . $this->string->upperCaseWords($stackVars[$i]['name'], '_', '');
                     $stackVars[$i]['variable'] = method_exists(
@@ -332,19 +388,33 @@ protected function getVariable($value, $default = '{no_value_defined}')
                         $stackVars[$i]['name']
                     );
                 } elseif ($stackVars[$i]['type'] == 'method') {
-                    // Calling of object method
-                    if (
-                            method_exists($stackVars[$i - 1]['variable'], $stackVars[$i]['name'])
-                            || substr($stackVars[$i]['name'], 0, 3) == 'get'
+                    // Calling of data object method
+                    if (method_exists($stackVars[$i - 1]['variable'], $stackVars[$i]['name'])
+                        || substr($stackVars[$i]['name'], 0, 3) == 'get'
                     ) {
                         $stackVars[$i]['args'] = $this->getStackArgs($stackVars[$i]['args']);
-                        $stackVars[$i]['variable'] = call_user_func_array(
-                            [$stackVars[$i - 1]['variable'], $stackVars[$i]['name']],
-                            $stackVars[$i]['args']
-                        );
+                        if ($this->isAllowedDataObjectMethod($stackVars[$i - 1]['variable'], $stackVars[$i]['name'])) {
+                            $stackVars[$i]['variable'] = call_user_func_array(
+                                [$stackVars[$i - 1]['variable'], $stackVars[$i]['name']],
+                                $stackVars[$i]['args']
+                            );
+                        }
                     }
                 }
                 $last = $i;
+            } elseif (isset($stackVars[$i - 1]['variable'])
+                      && is_object($stackVars[$i - 1]['variable'])
+                      && $stackVars[$i]['type'] == 'method'
+            ) {
+                // Calling object methods
+                $object = $stackVars[$i - 1]['variable'];
+                $method = $stackVars[$i]['name'];
+                if (method_exists($object, $method)) {
+                    $args = $this->getStackArgs($stackVars[$i]['args']);
+                    $this->validateVariableMethodCall($object, $method);
+                    $stackVars[$i]['variable'] = call_user_func_array([$object, $method], $args);
+                }
+                $last = $i;
             }
         }
 

lib/internal/Magento/Framework/Filter/Test/Unit/TemplateTest.php

@@ -6,17 +6,26 @@
 
 namespace Magento\Framework\Filter\Test\Unit;
 
+use Magento\Framework\Filter\Template;
+use Magento\Store\Model\Store;
+
 class TemplateTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @var \Magento\Framework\Filter\Template
+     * @var Template
      */
     private $templateFilter;
 
+    /**
+     * @var Store
+     */
+    private $store;
+
     protected function setUp()
     {
         $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
-        $this->templateFilter = $objectManager->getObject('Magento\Framework\Filter\Template');
+        $this->templateFilter = $objectManager->getObject(Template::class);
+        $this->store = $objectManager->getObject(Store::class);
     }
 
     public function testFilter()
@@ -191,4 +200,36 @@ public function varDirectiveDataProvider()
             ],
         ];
     }
+
+    /**
+     * Test filtering disallowed methods calls.
+     *
+     * @param string $method
+     * @dataProvider disallowedMethods
+     * @expectedException \InvalidArgumentException
+     *
+     * @return void
+     */
+    public function testDisallowedMethods(string $method)
+    {
+        $this->templateFilter->setVariables(['store' => $this->store]);
+        $this->templateFilter->filter('{{var store.' . $method . '()}}');
+    }
+
+    /**
+     * Data for testDisallowedMethods method.
+     *
+     * @return array
+     */
+    public function disallowedMethods()
+    {
+        return [
+            ['getResourceCollection'],
+            ['load'],
+            ['save'],
+            ['getCollection'],
+            ['getResource'],
+            ['getConfig'],
+        ];
+    }
 }