Lekje?

Jeroen Boersma

@srcoder

@Jeroen_Boersma

file_get_contents?!!

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

        $download = Mage::getBaseDir('media') . DS . 'downloads' . DS . $fileName;
        $this->_prepareDownloadResponse($fileName, file_get_contents($download));
    }

}

/**
 * curl http://test.site/mydownload/main/download/file/test.xml
 * --> /media/downloads/test.xml
 *
 * curl http://test.site/mydownload/main/download?file=../../app/etc/local.xml
 * --> /media/downloads/../../app/etc/local.xml
 * --> /app/etc/local.xml --> Oh no!
 *
 * curl -d 'file=../../../../../../../../../../../.../etc/passwd' http://test.site/mydownload/main/download
 * --> /media/downloads/../../../../../../../../../../../.../etc/passwd
 * --> /etc/passwd --> No No no!
 *
 */

getParam?!!

    /**
     * Retrieve a parameter
     *
     * Retrieves a parameter from the instance. Priority is in the order of
     * userland parameters (see {@link setParam()}), $_GET, $_POST. If a
     * parameter matching the $key is not found, null is returned.
     *
     * If the $key is an alias, the actual key aliased will be used.
     *
     * @param mixed $key
     * @param mixed $default Default value to use if key not found
     * @return mixed
     */
    public function getParam($key, $default = null)
    {
        $keyName = (null !== ($alias = $this->getAlias($key))) ? $alias : $key;

        $paramSources = $this->getParamSources();
        if (isset($this->_params[$keyName])) {
            return $this->_params[$keyName];
        } elseif (in_array('_GET', $paramSources) && (isset($_GET[$keyName]))) {
            return $_GET[$keyName];
        } elseif (in_array('_POST', $paramSources) && (isset($_POST[$keyName]))) {
            return $_POST[$keyName];
        }

        return $default;
    }

alternatives..

    public function getParam($key, $default = null){}

    public function getQuery($key, $default = null){}

    public function getPost($key, $default = null){}

securing...

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

        /**
         * Verify path before proceeding download
         * 
         * {DOCROOT}/media/downloads
         * {DOCROOT}/media/downloads/test.xml
         * 
         * /app/etc/local.xml
         * /etc/passwd
         */
        $checkPath = realpath(Mage::getBaseDir('media') . DS . 'downloads' . DS);
        $download = realpath(Mage::getBaseDir('media') . DS . 'downloads' . DS . $fileName);
        if (strpos($download, $checkPath) === false) {
            return $this->norouteAction();
        }

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

}

testing...

# Test for a common file, 200 OK?
# Don't try to hack sites without the module installed
# Silence is golden
curl http://test.site/js/mymodule/commonfile.js

# Test for vulnerability, should return 404
curl http://test.site/mydownload/main/download/file/../../app/Mage.php
# 200 OK will be vulnerable
# Will also know which Magento version

En dan..

• Maak een secret gist met alle informatie
• Neem contact op met de module bouwer en Magento
• Geef de kans aan de bouwer om te patchen en contact op te nemen met haar klanten
• Maak een public gist om ook iedereen te informeren zonder de eigenlijke hack
• Geef mensen de kans om te checken of lek zijn
• Verplaats naar Magereport.com
• White- vs blackhat

Tijd..

Houd er rekening mee dat het vinden, fixen en communiceren veel tijd kost.

Discussie ..

Hoe kunnen we Magento code veiliger maken?

Thanks