Everyday with a RCE, is a good day

In his SUPEE-8788 security advisory, Magento warns the users of possible Remote Code Execution attacks in unpatched versions.
Let's find out exactly what does it mean and how we can leverage it.

First of all we have to understand what was the flaw causing the RCE bug. As usual, the security advisor has very few details: this is a common pattern to avoid to disclose too many information to attackers.
This means that we have to analyze the bulletin, reverse engineer the patch and finally try to elaborate an educated guess on what's going on.

Vulnerability background

A Remote Code Execution is a very serious vulnerability: it means that an attacker could run any command remotely on the server, leading to a full compromise.
In PHP this usually happens with a call to the functions eval(), system() or unserialize() without sanitizing their arguments. The first two are easy to exploit and generally best practises discourage their usage; the last one is harder and requires more work to turn any bug into a security issue.

Exploiting unserialize(), a.k.a. POI

If unfiltered data is passed to the unserialize() function, we could have a PHP Object Injection vulnerability, where external code is injected inside the current code. This opens a complete new world, since we can "tweak" the original execution until we achieve something completely different.
As previously stated, this isn't something straightforward, but requires the attacker to hop on several objects, creating a full chain of gadgets.
This topic is quite large, you can find more resources and examples here and here. Long story short, this is what we you should keep in mind about the POI vulnerability:

In order to exploit a POI vulnerability, you have to find an unfiltered unserialize() function call. Once you can inject your code, you have to chain several object methods until you can trigger the exploit (SQL injection, write a file on filesystem etc etc)

Analyzing the patch

Now that we got some background info, we know what to search: we will have to search for usages of the unserialize() function all along the code base. However this could be an awful job, what if we do something more clever?
For example, we know that its usage could trigger a RCE exploit, so it should be included inside the latest changes. We could run the diff command on those two versions, but Magento kindly provide us a patch file ready to use.
Side note: there is a GitHub mirror repository, but I'd prefer official sources since sometimes deployed packages are slightly different.
A quick search for the unserialize function returned interesting results:

@@ -70,7 +70,13 @@ class Mage_Payment_Block_Info_Checkmo extends Mage_Payment_Block_Info
-        $details = @unserialize($this->getInfo()->getAdditionalData());
+        $details = false;
+        try {
+            $details = Mage::helper('core/unserializeArray')
+                ->unserialize($this->getInfo()->getAdditionalData());
+        } catch (Exception $e) {
+            Mage::logException($e);
+        }

@@ -53,6 +53,30 @@ class Mage_Paypal_Model_Resource_Payment_Transaction extends Mage_Core_Model_Res
     }

     /**
+     * Unserialize Varien_Object field in an object
+     *
+     * @param Mage_Core_Model_Abstract $object
+     * @param string $field
+     * @param mixed $defaultValue
+     */
+    protected function _unserializeField(Varien_Object $object, $field, $defaultValue = null)
+    {
+        $value = $object->getData($field);
+        if (empty($value)) {
+            $object->setData($field, $defaultValue);
+        } elseif (!is_array($value) && !is_object($value)) {
+            $unserializedValue = false;
+            try {
+                $unserializedValue = Mage::helper('core/unserializeArray')
+                    ->unserialize($value);
+            } catch (Exception $e) {
+                Mage::logException($e);
+            }
+            $object->setData($field, $unserializedValue);
+        }
+    }

Oh boy! Someone is really scared about this unserialize() function! They replaced all native calls with their helper method that filters the output. So my hunch was correct after all.

Hunting for a crack in the wall

Ok, so now we have a list of affected files. All I have to do is to research how I can supply my own data to any of those classes and exploit the vulnerability. Looking at the security bulletin, APPSEC-1484 - Remote Code Execution in checkout is a very juicy candidate.
So I start searching...
and searching...
and searching...

Until I decided to gave up :(
That was a bummer, but I spent countless nights trying to find an entry point to exploit without any luck.

Hunting for a crack behind the wall

Luckily, there are several other interesting (and maybe easier) candidates that I could test; for example APPSEC-1375 - Remote Code Execution in admin. Yes, it requires being already logged inside the admin area, but this could be our starting point.
If you remember how POI works, we need two things for a successful exploitation:

1. An unfiltered usage of unserialize()
2. A chain of gadgets until we can finally execute our own code

So I thought to find an easier target to address the first part and focus on the second one: if there's no way to build a gadget chain, there's no use to continue searching.

I started looking at the patch file, until I saw these lines:

diff --git app/code/core/Mage/Dataflow/Model/Profile.php app/code/core/Mage/Dataflow/Model/Profile.php
index f7232cf..6534dcf 100644
--- app/code/core/Mage/Dataflow/Model/Profile.php
+++ app/code/core/Mage/Dataflow/Model/Profile.php
@@ -127,7 +131,13 @@ class Mage_Dataflow_Model_Profile extends Mage_Core_Model_Abstract
     protected function _afterSave()
     {
         if (is_string($this->getGuiData())) {
-            $this->setGuiData(unserialize($this->getGuiData()));
+            try {
+                $guiData = Mage::helper('core/unserializeArray')
+                    ->unserialize($this->getGuiData());
+                $this->setGuiData($guiData);
+            } catch (Exception $e) {
+                Mage::logException($e);
+            }
         }

Mhm... seems interesting. When gui_data is set? Can we inject it?
Well, it's quite easy to test. Let's attach PHP XDebugger and follow the flow.
Long story short, Magento does not validate the gui_data input field while saving and after the save method it tries to unserialize what we have just passed.
This is exactly what we where looking for!

gui_data contains the string "foobar" instead of an array of fields

gui data now is false, since unserialize finds an invalid string.

Bend the execution flow

Once found an injection point, the next step was to actually send some code and execute it. If you read the above links about POI, you should know that this is as hard as finding the original exploit.
Instead of searching trough the whole code, I decided to spend some time googling.
It turns out that Magento 2.0.6 suffered of RCE, too; you can find the exploit code here on exploit-db.com.
The interesting thing is that Magento 1 and Magento 2 classes are (for the moment) very similar, so if that exploit was working, we can adapt it and use it in our scenario.
I changed some class names and import different objects, but the whole concept is to build the following chain:

1. Leverage Credis_Client::__destruct() function, that will call the close() method on his protected member $redis
2. Inject Mage_Sales_Model_Order_Payment_Transaction inside Credis_Client::$redis, so we will call the close() method on it
3. Mage_Sales_Model_Order_Payment_Transaction::close() will call the save() method on its resource
4. Inject Varien_Simplexml_Config_Cache_File inside Mage_Sales_Model_Order_Payment_Transaction::$_resource, so we will call the save method on it
5. Varien_Simplexml_Config_Cache_File::save() will write contents to disk

You can find a gist with the whole code here.

Another delusion

Sadly it turns out that Magento 1 implements a singleton pattern, so even if we put an object inside Mage_Sales_Model_Order_Payment_Transaction, we will go trough Mage::getResourceSingleton() method, which simply breaks the execution.
On top of that, method beginTransaction() would be called on the resource, and that's simply something we do not have.

Conclusions

Honestly I wrote this blog post since I need some closure on this issue.
I'll save you the painstaking task to review all the possible chains between different classes. I spent countless hours on this, since it seems there are so many injection points and the opportunity to execute a RCE on one of the most famous e-commerce platform was too juicy.
Seems it was too good to be true :( (at least for me).
If anyone has any suggestion, can contact me or leave a comment here!