Time to Patch Magento Again

Published by John on January 20, 2016 Under Magento

Magento released two patches today. One is a patch for USPS shipping to take into account some changes to method names of shipping lines, like changing from calling it Standard Post to Retail Ground. The other is a rather serious security patch, which fixes several XSS vulnerabilities in Magento, as well as un-privileged escalations. The USPS patch is SUPEE-7616 and the XSS/Security Patch is SUPEE-7405, which can be downloaded here (see the release archive section.)

As is usually the case, the XSS vulnerabilities appear to be mostly the result of not properly escaping user input during the order process. There are several vulnerabilities that allow users to input malicious input during the order process in their Email, an order comment, and the HTTP_X_FORWARDED_FOR header of their request, which result in javascript being executed when viewing the order. This could in turn lead to further issues, although these ones do seem to at least require an order to be placed first, so there is some comfort there.

In any-case Patch Patch Patch!!!

Issues with USPS SUPEE-7616

While applying the USPS SUPEE-7616 Patch on a Magento site running 1.8.1.0, I ran into an issue with the first part of the patch failing. The error was:

Hunk #1 FAILED at 544.
Hunk #2 succeeded at 569 (offset -10 lines).
Hunk #3 succeeded at 600 (offset -10 lines).
Hunk #4 succeeded at 611 (offset -10 lines).
Hunk #5 succeeded at 646 (offset -10 lines).
Hunk #6 succeeded at 677 (offset -10 lines).
Hunk #7 succeeded at 726 (offset -10 lines).
Hunk #8 succeeded at 765 (offset -10 lines).
Hunk #9 succeeded at 816 (offset -10 lines).
Hunk #10 succeeded at 839 (offset -10 lines).
Hunk #11 succeeded at 1405 (offset -10 lines).
1 out of 11 hunks FAILED -- saving rejects to file app/code/core/Mage/Usa/Model/Shipping/Carrier/Usps.php.rej

I do not believe this file had been modified and I compared it against a backup, as well as downloading a clean copy of 1.8.10 and comparing against that. In both cases, it appeared the same. Rather than play with it too much, I ended up just manually applying that first Hunk, as it was really just a single line. After applying it, I deleted that section.

First, change ‘Standard Post’ in the below, to ‘Retail Ground’

'4'      => Mage::helper('usa')->__('Retail Ground'),

Then, delete the below from the patch and the rest of it should apply correctly.

@@ -544,7 +544,7 @@ class Mage_Usa_Model_Shipping_Carrier_Usps
                  '1'      => Mage::helper('usa')->__('Priority Mail'),
                  '2'      => Mage::helper('usa')->__('Priority Mail Express Hold For Pickup'),
                  '3'      => Mage::helper('usa')->__('Priority Mail Express'),
-                 '4'      => Mage::helper('usa')->__('Standard Post'),
+                 '4'      => Mage::helper('usa')->__('Retail Ground'),
                  '6'      => Mage::helper('usa')->__('Media Mail Parcel'),
                  '7'      => Mage::helper('usa')->__('Library Mail Parcel'),
                  '13'     => Mage::helper('usa')->__('Priority Mail Express Flat Rate Envelope'),

Issues with USPS SUPEE-7405

On one of my sites running Centos 6, which uses PHP 5.3, I ran into an issue with patch 7405, as the patch uses a non-supported version of declaring an array in PHP 5.3.

The error was thrown when displaying an order and was:

Parse error:  syntax error, unexpected '[' in /public_html/shop/app/code/core/Mage/Adminhtml/Helper/Sales.php on line 124

This happens because of the following:

$links = [];

Unfortunately, declaring an array is only available on PHP 5.4+, so on a Centos Site running 5.3, this needs to be updated to the following:

$links = array();

A look at some of the vulnerabilities

I took a quick look at Patch 7405 and it contains several interesting snippets. Most of the XSS vulnerabilities appear to be addressed via proper escaping using the escapeHtml() function.

For example, the below occurs in various places with several

-                <td class="value"><strong><?php echo $_order->getRemoteIp(); echo ($_order->getXForwardedFor())?' (' . $this->escapeHtml($_order->getXForwardedFor()) . ')':''; ?></strong></td>
+                <td class="value"><strong><?php echo $this->escapeHtml($_order->getRemoteIp()); echo ($_order->getXForwardedFor())?' (' . $this->escapeHtml($_order->getXForwardedFor()) . ')':''; ?></strong></td>

The above adds an ‘escapeHtml’ call when printing the order IP, as per the patch “In certain configurations, Magento uses the HTTP_X_FORWARDED_FOR header as the customer IP.”

From looking through the patch, proper escaping was not happening on several fields. Another interesting change was a new function called escapeHtmlWithLinks, which gets used to escape the user’s order comment, while still allowing for links. It preforms some regex to parse out links and then adds them back, without only a href, so to avoid things like onclick, which could result in XSS.

The review changes are kind of interesting too, they added a check to ensure the user is logged in and also a new function called _cropReviewData, which limits what fields are returned from a review to only: ‘detail’, ‘title’, ‘nickname’

No Comments |