Hi,

I have discovered a vulnerability that might lead to code execution in
Contao CMS <= 3.2.4
Contao CMS <= 3.2.4 does not properly validate user input in several
locations which is then passed directly into PHP's unserialize.

This has been fixed in Contao 3.2.5 as per commit:
https://github.com/contao/core/commit/8c9cb044bdc887a8202bb65a64545c025664f957
and
https://github.com/contao/core/commit/1717336598fdcf1ed3f4ad488e140147cb31516d

Announcements can be found at

https://contao.org/en/news/contao-3_2_5.html

https://contao.org/en/news/contao-2_11_14.html

Thanks to the Contao developers for being so responsive.
The full report can be found at my repo in
https://github.com/pedrib/PoC/blob/master/contao-3.2.4.txt

Regards,

Pedro Ribeiro
Agile Information Security


------
Proof of concept:

> Vulnerabilities in Contao 3.2.4
> Discovered by Pedro Ribeiro (pedrib@gmail.com) of Agile Information Security

====================================================================
Vulnerability: PHP object injection leading to all kinds of badness
File(line): contao-core-3.2.4/system/modules/core/drivers/DC_Table.php(149)
File(line): contao-core-3.2.4/system/modules/core/drivers/DC_Folder.php(124)
File(line): contao-core-3.2.4/system/modules/core/widgets/KeyValueWizard.php(73)
File(line): Potentially many others that call deserialize with user input
Code snippet:

contao-core-3.2.4/system/modules/core/drivers/DC_Table.php(149):
if (\Input::post('FORM_SUBMIT') == 'tl_select')
{
$ids = deserialize(\Input::post('IDS'));

if (!is_array($ids) || empty($ids))
{
$this->reload();
}

$session = $this->Session->getData();
$session['CURRENT']['IDS'] = deserialize(\Input::post('IDS'));


contao-core-3.2.4/system/modules/core/drivers/DC_Folder.php(124):
if (\Input::post('FORM_SUBMIT') == 'tl_select')
{
$ids = deserialize(\Input::post('IDS'));


contao-core-3.2.4/system/modules/core/widgets/KeyValueWizard.php(73)
public function validate()
{
$mandatory = $this->mandatory;
$options = deserialize($this->getPost($this->strName));


deserialize from functions.php (starting at line 280):
/**
 * Return an unserialized array or the argument
 * @param mixed
 * @param boolean
 * @return mixed
 */
function deserialize($varValue, $blnForceArray=false)
{
if (is_array($varValue))
{
return $varValue;
}

if (!is_string($varValue))
{
return $blnForceArray ? (($varValue === null) ? array() : array($varValue)) : $varValue;
}
elseif (trim($varValue) == '')
{
return $blnForceArray ? array() : '';
}

$varUnserialized = @unserialize($varValue);

if (is_array($varUnserialized))
{
$varValue = $varUnserialized;
}
elseif ($blnForceArray)
{
$varValue = array($varValue);
}

return $varValue;
}


The Input class does good validation on POST and GET variables for XSS, but in this case it provides no protection because the malicious data is a well formed PHP object.

post from Input class:
public static function post($strKey, $blnDecodeEntities=false)
{
$strCacheKey = $blnDecodeEntities ? 'postDecoded' : 'postEncoded';

if (!isset(static::$arrCache[$strCacheKey][$strKey]))
{
$varValue = static::findPost($strKey);

if ($varValue === null)
{
return $varValue;
}

$varValue = static::stripSlashes($varValue);
$varValue = static::decodeEntities($varValue);
$varValue = static::xssClean($varValue, true);
$varValue = static::stripTags($varValue);

if (!$blnDecodeEntities)
{
$varValue = static::encodeSpecialChars($varValue);
}

static::$arrCache[$strCacheKey][$strKey] = $varValue;
}

return static::$arrCache[$strCacheKey][$strKey];
}


getPost from Widget class (starting at line 722):
/**
 * Find and return a $_POST variable
 *
 * @param string $strKey The variable name
 *
 * @return mixed The variable value
 */
protected function getPost($strKey)
{
$strMethod = $this->allowHtml ? 'postHtml' : 'post';

if ($this->preserveTags)
{
$strMethod = 'postRaw';
}

// Support arrays (thanks to Andreas Schempp)
$arrParts = explode('[', str_replace(']', '', $strKey));

if (!empty($arrParts))
{
$varValue = \Input::$strMethod(array_shift($arrParts), $this->decodeEntities);

foreach($arrParts as $part)
{
if (!is_array($varValue))
{
break;
}

$varValue = $varValue[$part];
}

return $varValue;
}

return \Input::$strMethod($strKey, $this->decodeEntities);
}


There are a few magic methods that seem exploitable.
For example on contao-core-3.2.4/system/modules/core/classes/FrontendUser.php, it might be possible to overwrite the admin session with another session by manipulating the intId parameter (which is the userID):
public function __destruct()
{
$session = $this->Session->getData();

if (!isset($_GET['pdf']) && !isset($_GET['file']) && !isset($_GET['id']) && $session['referer']['current'] != \Environment::get('requestUri'))
{
$session['referer']['last'] = $session['referer']['current'];
$session['referer']['current'] = substr(\Environment::get('requestUri'), strlen(TL_PATH) + 1);
}

$this->Session->setData($session);

if ($this->intId)
{
$this->Database->prepare("UPDATE " . $this->strTable . " SET session=? WHERE id=?")
->execute(serialize($session), $this->intId);
}
}


Overwriting configuration files in contao-core-3.2.4/system/modules/core/library/Contao/Config.php

/**
* Automatically save the local configuration
*/
public function __destruct()
{
if ($this->blnIsModified)
{
$this->save();
}
}


public function save()
{
if ($this->strTop == '')
{
$this->strTop = '<?php';
}

$strFile = trim($this->strTop) . "\n\n";
$strFile .= "### INSTALL SCRIPT START ###\n";

foreach ($this->arrData as $k=>$v)
{
$strFile .= "$k = $v\n";
}

$strFile .= "### INSTALL SCRIPT STOP ###\n";
$this->strBottom = trim($this->strBottom);

if ($this->strBottom != '')
{
$strFile .= "\n" . $this->strBottom . "\n";
}

$strTemp = md5(uniqid(mt_rand(), true));

// Write to a temp file first
$objFile = fopen(TL_ROOT . '/system/tmp/' . $strTemp, 'wb');
fputs($objFile, $strFile);
fclose($objFile);

// Make sure the file has been written (see #4483)
if (!filesize(TL_ROOT . '/system/tmp/' . $strTemp))
{
\System::log('The local configuration file could not be written. Have your reached your quota limit?', __METHOD__, TL_ERROR);
return;
}

// Then move the file to its final destination
$this->Files->rename('system/tmp/' . $strTemp, 'system/config/localconfig.php');


Arbitrary file deletion in contao-core-3.2.4/system/modules/core/library/Contao/ZipWriter.php:
public function __destruct()
{
if (is_resource($this->resFile))
{
@fclose($this->resFile);
}

if (file_exists($this->strTemp))
{
@unlink($this->strTemp);
}


Again arbitrary file deletion in contao-core-3.2.4/system/modules/core/vendor/swiftmailer/classes/Swift/ByteStream/TemporaryFileByteStream.php:
    public function __destruct()
    {
        if (file_exists($this->getPath())) {
            @unlink($this->getPath());
        }
    }


Comment:
User input is passed directly into unserialize(), possibly leading to code execution.

References:
https://www.owasp.org/index.php/PHP_Object_Injection
http://www.alertlogic.com/writing-exploits-for-exotic-bug-classes/
http://www.suspekt.org/downloads/POC2009-ShockingNewsInPHPExploitation.pdf
http://vagosec.org/2013/12/wordpress-rce-exploit/