5

Magento released last patch for M1:

https://magento.com/tech-resources/download#download2374

Haven't looked into the patch yet, it looks like the community is unaware there is a new patch out for M1.

Please share any issue you have found with this patch!

  • I am not finding this patch download on magento site, anyone knows from where we can download it? – Prashant Jul 16 at 12:20
9

I just patched 1.9.3.6, here's a quick summary of what's going on:

diff --git a/app/code/core/Mage/Adminhtml/Block/Widget/Form.php b/app/code/core/Mage/Adminhtml/Block/Widget/Form.php
index c00597b9..87bc93b0 100644
--- a/app/code/core/Mage/Adminhtml/Block/Widget/Form.php
+++ b/app/code/core/Mage/Adminhtml/Block/Widget/Form.php
@@ -190,7 +190,7 @@ class Mage_Adminhtml_Block_Widget_Form extends Mage_Adminhtml_Block_Widget
                         'label'     => $attribute->getFrontend()->getLabel(),
                         'class'     => $attribute->getFrontend()->getClass(),
                         'required'  => $attribute->getIsRequired(),
-                        'note'      => $attribute->getNote(),
+                        'note'      => $this->escapeHtml($attribute->getNote()),
                     )
                 )
                 ->setEntityAttribute($attribute);

Here Magento wraps the $attribute->getNote() in an escapeHtml function, escaping any malicious code which is entered in the note attribute (which can get filled through the admin). In order to abuse this vulnerability, the attacker should have had access to the admin or the database.

diff --git a/app/code/core/Mage/Adminhtml/controllers/Catalog/Product/AttributeController.php b/app/code/core/Mage/Adminhtml/controllers/Catalog/Product/AttributeController.php
index ccbf26cd..7fc9c7a1 100644
--- a/app/code/core/Mage/Adminhtml/controllers/Catalog/Product/AttributeController.php
+++ b/app/code/core/Mage/Adminhtml/controllers/Catalog/Product/AttributeController.php
@@ -287,6 +287,10 @@ class Mage_Adminhtml_Catalog_Product_AttributeController extends Mage_Adminhtml_
                 $data['apply_to'] = array();
             }
 
+            if ($model) {
+                $data['entity_type_id'] = $model->getEntityTypeId();
+            }
+
             //filter
             $data = $this->_filterPostData($data);
             $model->addData($data);

This is in the save action for an attribute in the admin. When a source model has been set for the attribute at hand, the entity_type_id value is filled. This is because the entity_type_id is not passed to the controller action through the request since there is no input field for it in the admin.

diff --git a/app/code/core/Mage/Eav/Model/Attribute.php b/app/code/core/Mage/Eav/Model/Attribute.php
index 1a871a76..c4827f52 100644
--- a/app/code/core/Mage/Eav/Model/Attribute.php
+++ b/app/code/core/Mage/Eav/Model/Attribute.php
@@ -116,7 +116,7 @@ abstract class Mage_Eav_Model_Attribute extends Mage_Eav_Model_Entity_Attribute
         if (is_array($rules)) {
             return $rules;
         } else if (!empty($rules)) {
-            return unserialize($rules);
+            return Mage::helper('core/unserializeArray')->unserialize($rules);
         }
         return array();

This is in the attribute model to get the validation rules for the attribute value. This now uses the Unserialize Parser (in ./lib/Unserialize/Parser.php) which was introduced with SUPEE-8788 v1 (and fixed in SUPEE-8788 v2). This parser strips out null values to make the unserialization process more secure.

These 3 are very low-risk changes so I'm not expecting any issues with this patch whatsoever (unless you missed SUPEE-8788).

| improve this answer | |
  • "When a source model has been set for the attribute at hand, the entity_type_id value is filled." What do you mean by "source model", not the "source_model" property I presume? Whether an attribute has a source_model or not, the the entity_type_id is never passed to the controller action. So it looks like this could be abused to move the attribute to a different entity type. – Aad Mathijssen Jun 24 at 15:52
  • 3
    "So it looks like this could be abused to move the attribute to a different entity type" Thats exactly what this was. I moved attributes from one entity type to the other to trigger RCE – Luke Rodgers Jun 24 at 17:28
  • 1
    One important part, we should not forget is, that the parser does NOT call __unserialize and/or __wakeup (From the docs: "If the variable being unserialized is an object, after successfully reconstructing the object PHP will automatically attempt to call the __unserialize() or __wakeup() methods (if one exists).") php.net/manual/en/function.unserialize.php – Fabian Blechschmidt Jun 29 at 9:42
2

Problem #1 with this patch is finding the official download link as Adobe seems to be removing hints of Magento1 from their site.

Update from comments

Official download of patches here: https://github.com/m-a-org/magento-patches

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.