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

Should changes from SUPEE-11086 be applied to Ho_Import_Model_Template_Filter::_getVariable ? #162

Open
hostep opened this issue Apr 29, 2019 · 1 comment

Comments

@hostep
Copy link

hostep commented Apr 29, 2019

Hi guys

I'm currently patching some M1 shops with the security patch SUPEE-11086.
I always write some custom script to see if any of the fixes done in a security patch should be applied to the custom code we added to a shop, and my script detected this module.

SUPEE-11086 changes the following lines in the file lib/Varien/Filter/Template.php:

diff --git a/lib/Varien/Filter/Template.php b/lib/Varien/Filter/Template.php
index e56960f5..7af4f2a7 100644
--- a/lib/Varien/Filter/Template.php
+++ b/lib/Varien/Filter/Template.php
@@ -289,6 +289,8 @@ class Varien_Filter_Template implements Zend_Filter_Interface
         $stackVars = $tokenizer->tokenize();
         $result = $default;
         $last = 0;
+        /** @var $emailPathValidator Mage_Adminhtml_Model_Email_PathValidator */
+        $emailPathValidator = $this->getEmailPathValidator();
         for($i = 0; $i < count($stackVars); $i ++) {
             if ($i == 0 && isset($this->_templateVars[$stackVars[$i]['name']])) {
                 // Getting of template value
@@ -305,9 +307,13 @@ class Varien_Filter_Template implements Zend_Filter_Interface
                     if (method_exists($stackVars[$i-1]['variable'], $stackVars[$i]['name'])
                         || substr($stackVars[$i]['name'], 0, 3) == 'get'
                     ) {
+                        $isEncrypted = false;
+                        if ($stackVars[$i]['name'] == 'getConfig') {
+                            $isEncrypted = $emailPathValidator->isValid($stackVars[$i]['args']);
+                        }
                         $stackVars[$i]['variable'] = call_user_func_array(
                             array($stackVars[$i-1]['variable'], $stackVars[$i]['name']),
-                            $stackVars[$i]['args']
+                            !$isEncrypted ? $stackVars[$i]['args'] : array(null)
                         );
                     }
                 }
@@ -322,4 +328,14 @@ class Varien_Filter_Template implements Zend_Filter_Interface
         Varien_Profiler::stop("email_template_proccessing_variables");
         return $result;
     }
+
+    /**
+     * Retrieve model object
+     *
+     * @return Mage_Core_Model_Abstract
+     */
+    protected function getEmailPathValidator()
+    {
+        return Mage::getModel('adminhtml/email_pathValidator');
+    }
 }

And since this module inherits from that class and rewrites the _getVariable method, the same changes might be needed here as well?

I don't really understand yet what security issue this is fixing and if this can be exploited using an import, but who knows ...

Thanks!

@hostep hostep changed the title Should changes from SUPEE-11086 be applied to Ho_Import_Model_Template_Filter::_getVariable Should changes from SUPEE-11086 be applied to Ho_Import_Model_Template_Filter::_getVariable ? Apr 29, 2019
@hostep
Copy link
Author

hostep commented Apr 29, 2019

This might not be very important, since I don't think this method is called anywhere?
It was added in bb70580, but I don't see where that templateEngine method gets called? It doesn't seem to be called in this module and also nowhere in core Magento as far as I can tell...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant