Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 2.15.2 to staging #7755

Merged
merged 24 commits into from Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8abdcc7
Merge branch 'release-2.15.1'
alanhartless Mar 25, 2019
0dfffae
Prevented open redirect vulnerability
alanhartless Mar 8, 2019
30d3dab
Use a helper to support tests
alanhartless Mar 8, 2019
de99722
Release 2.15.2 beta (#7580)
kuzmany Jun 3, 2019
f2b1008
Escape validation error messages to avoid HTML tag rendering in the UI
escopecz Jun 14, 2019
5d4d62c
Merge pull request #50 from mautic-inc/escape-validation-errors
Jun 25, 2019
b32fa96
Merge pull request #47 from mautic-inc/security.open-redirect
Jul 26, 2019
4114df4
Disable unserializing of classes. We should unserialize only array data.
escopecz Jul 29, 2019
3cc288e
Allow unserializing of Swift_Message objects for email queue processing
escopecz Jul 29, 2019
d94668c
Escape validation error messages to avoid HTML tag rendering in the UI
kuzmany Jul 31, 2019
1ce0094
Prevent open redirect vulnerability
kuzmany Jul 31, 2019
a90ddf5
Disable unserializing of classes. We should unserialize only array data
kuzmany Jul 31, 2019
615a2bf
Escape values in Lead and Report bundles to prevent html formatting
kuzmany Jul 31, 2019
2f7334a
Bump version 2.15.2
kuzmany Jul 31, 2019
c33ff9a
Assets regenerate
kuzmany Jul 31, 2019
452b40c
Merge remote-tracking branch 'origin/master' into release-2.15.2
kuzmany Jul 31, 2019
ad708fd
Fix travis builds on staging
kuzmany Jul 31, 2019
d8b2d73
New serializer helper that will care about secure usage and different…
escopecz Aug 1, 2019
6ae81bf
Use secure serializer helper over the native unserialize method
escopecz Aug 1, 2019
e94dfb7
Manual merge
escopecz Aug 1, 2019
7dfb25e
More accurate comment
escopecz Aug 1, 2019
e9100c1
Even more accurate comment
escopecz Aug 1, 2019
8affe0e
Bump version
kuzmany Aug 1, 2019
7302a31
Update .htaccess
kuzmany Aug 1, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions .htaccess
Expand Up @@ -96,3 +96,22 @@
</IfModule>
</IfModule>
</IfModule>

# Denie access via HTTP requests to all PHP files.
<Files "*.php">
Require all denied
</Files>

# Except those whitelisted bellow.
<Files "index.php">
Require all granted
</Files>
<Files "index_dev.php">
Require all granted
</Files>
<Files "filemanager.php">
Require all granted
</Files>
<Files "upgrade.php">
Require all granted
</Files>
2 changes: 1 addition & 1 deletion app/bundles/CampaignBundle/Views/Campaign/leads.html.php
Expand Up @@ -24,7 +24,7 @@
<?php if ($preferred == 'gravatar' || empty($preferred)) : ?>
<?php $img = $view['gravatar']->getImage($item['email'], '250'); ?>
<?php else : ?>
<?php $socialData = unserialize($item['social_cache']); ?>
<?php $socialData = \Mautic\CoreBundle\Helper\Serializer::decode($item['social_cache']); ?>
<?php $img = (!empty($socialData[$preferred]['profile']['profileImage'])) ? $socialData[$preferred]['profile']['profileImage'] : $view['gravatar']->getImage($item['email'], '250'); ?>
<?php endif; ?>
<img class="img img-responsive" src="<?php echo $img; ?>" />
Expand Down
2,587 changes: 1,343 additions & 1,244 deletions app/bundles/CoreBundle/Assets/js/libraries/3a.chosen.jquery.js

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions app/bundles/CoreBundle/Config/config.php
Expand Up @@ -575,6 +575,12 @@
'mautic.helper.file_properties' => [
'class' => \Mautic\CoreBundle\Helper\FileProperties::class,
],
'mautic.helper.trailing_slash' => [
'class' => \Mautic\CoreBundle\Helper\TrailingSlashHelper::class,
'arguments' => [
'mautic.helper.core_parameters',
],
],
],
'menus' => [
'mautic.menu.main' => [
Expand Down
9 changes: 4 additions & 5 deletions app/bundles/CoreBundle/Controller/CommonController.php
Expand Up @@ -20,6 +20,7 @@
use Mautic\CoreBundle\Helper\CoreParametersHelper;
use Mautic\CoreBundle\Helper\DataExporterHelper;
use Mautic\CoreBundle\Helper\InputHelper;
use Mautic\CoreBundle\Helper\TrailingSlashHelper;
use Mautic\CoreBundle\Model\AbstractCommonModel;
use Mautic\UserBundle\Entity\User;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
Expand Down Expand Up @@ -258,12 +259,10 @@ public function delegateRedirect($url)
*/
public function removeTrailingSlashAction(Request $request)
{
$pathInfo = $request->getPathInfo();
$requestUri = $request->getRequestUri();
/** @var TrailingSlashHelper $trailingSlashHelper */
$trailingSlashHelper = $this->get('mautic.helper.trailing_slash');

$url = str_replace($pathInfo, rtrim($pathInfo, ' /'), $requestUri);

return $this->redirect($url, 301);
return $this->redirect($trailingSlashHelper->getSafeRedirectUrl($request), 301);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion app/bundles/CoreBundle/Helper/ClickthroughHelper.php
Expand Up @@ -46,6 +46,6 @@ public static function decodeArrayFromUrl($string, $urlDecode = true)
throw new \InvalidArgumentException(sprintf('The string %s is not a serialized array.', $decoded));
}

return unserialize($decoded);
return Serializer::decode($decoded);
}
}
2 changes: 1 addition & 1 deletion app/bundles/CoreBundle/Helper/EncryptionHelper.php
Expand Up @@ -104,7 +104,7 @@ public function decrypt($data, $mainDecryptOnly = false)
return false;
}
try {
return unserialize($availableCipher->decrypt($encryptedMessage, $this->key, $initVector));
return Serializer::decode($availableCipher->decrypt($encryptedMessage, $this->key, $initVector));
} catch (InvalidDecryptionException $ex) {
}
$mainTried = true;
Expand Down
35 changes: 35 additions & 0 deletions app/bundles/CoreBundle/Helper/Serializer.php
@@ -0,0 +1,35 @@
<?php

/*
* @copyright 2019 Mautic Contributors. All rights reserved
* @author Mautic
*
* @link http://mautic.org
*
* @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
*/

namespace Mautic\CoreBundle\Helper;

class Serializer
{
/**
* Unserializing a string can be a security vulnerability as it can contain classes that can execute a PHP code.
* PHP >=7 has the `['allowed_classes' => false]` option to disable classes altogether or whitelist those needed.
* PHP <7 do not accept the second parameter, throw warning and return false so we have to handle it diffenetly.
* This helper method is secure for PHP >= 7 by default and handle all PHP versions.
*
* @param string $serializedString
* @param array $options
*
* @return mixed
*/
public static function decode($serializedString, array $options = ['allowed_classes' => false])
{
if (version_compare(phpversion(), '7.0.0', '<')) {
return unserialize($serializedString);
}

return unserialize($serializedString, $options);
}
}
45 changes: 45 additions & 0 deletions app/bundles/CoreBundle/Helper/TrailingSlashHelper.php
@@ -0,0 +1,45 @@
<?php

/*
* @copyright 2019 Mautic Inc. All rights reserved
* @author Mautic, Inc.
*
* @link https://www.mautic.com
*
* @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
*/

namespace Mautic\CoreBundle\Helper;

use Symfony\Component\HttpFoundation\Request;

class TrailingSlashHelper
{
/**
* @var CoreParametersHelper
*/
private $coreParametersHelper;

/**
* TrailingSlashHelper constructor.
*
* @param CoreParametersHelper $coreParametersHelper
*/
public function __construct(CoreParametersHelper $coreParametersHelper)
{
$this->coreParametersHelper = $coreParametersHelper;
}

/**
* @param Request $request
*
* @return string
*/
public function getSafeRedirectUrl(Request $request)
{
$siteUrl = $this->coreParametersHelper->getParameter('site_url');
$pathInfo = substr($request->getPathInfo(), 0, -1);

return $siteUrl.$pathInfo;
}
}
3 changes: 2 additions & 1 deletion app/bundles/CoreBundle/Templating/Helper/FormatterHelper.php
Expand Up @@ -13,6 +13,7 @@

use Mautic\CoreBundle\Helper\AppVersion;
use Mautic\CoreBundle\Helper\InputHelper;
use Mautic\CoreBundle\Helper\Serializer;
use Symfony\Component\Templating\Helper\Helper;
use Symfony\Component\Translation\TranslatorInterface;

Expand Down Expand Up @@ -68,7 +69,7 @@ public function _($val, $type = 'html', $textOnly = false, $round = 1)
case 'array':
if (!is_array($val)) {
//assume that it's serialized
$unserialized = unserialize($val);
$unserialized = Serializer::decode($val);
if ($unserialized) {
$val = $unserialized;
}
Expand Down
@@ -0,0 +1,94 @@
<?php

/*
* @copyright 2019 Mautic Inc. All rights reserved
* @author Mautic, Inc.
*
* @link https://www.mautic.com
*
* @license GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
*/

namespace Mautic\CoreBundle\Tests\Helper;

use Mautic\CoreBundle\Helper\CoreParametersHelper;
use Mautic\CoreBundle\Helper\TrailingSlashHelper;
use Symfony\Component\HttpFoundation\Request;

class TrailingSlashHelperTest extends \PHPUnit_Framework_TestCase
{
/**
* @var CoreParametersHelper|\PHPUnit_Framework_MockObject_MockObject
*/
private $coreParametersHelper;

protected function setUp()
{
$this->coreParametersHelper = $this->createMock(CoreParametersHelper::class);
$this->coreParametersHelper->method('getParameter')
->with('site_url')
->willReturn('https://test.com');
}

public function testOpenRedirectIsNotPossible()
{
$server = [
'HTTP_HOST' => 'test.com',
'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.56 Safari/537.36',
'SERVER_NAME' => 'test.com',
'SERVER_ADDR' => '::1',
'SERVER_PORT' => '80',
'REMOTE_ADDR' => '::1',
'DOCUMENT_ROOT' => null,
'REQUEST_SCHEME' => 'http',
'REMOTE_PORT' => '80',
'REDIRECT_URL' => '/google.com/',
'SERVER_PROTOCOL' => 'HTTP/1.1',
'REQUEST_METHOD' => 'GET',
'QUERY_STRING' => '',
'REQUEST_URI' => '//google.com/',
'SCRIPT_NAME' => '/index.php',
'PHP_SELF' => '/index.php',
];

$request = new Request([], [], [], [], [], $server);

// google.com should not be returned as the URL
$this->assertEquals('https://test.com//google.com', $this->getHelper()->getSafeRedirectUrl($request));
}

public function testMauticUrlWithTrailingSlashIsGeneratedCorrectly()
{
$server = [
'HTTP_HOST' => 'test.com',
'HTTP_USER_AGENT' => 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.56 Safari/537.36',
'SERVER_NAME' => 'test.com',
'SERVER_ADDR' => '::1',
'SERVER_PORT' => '80',
'REMOTE_ADDR' => '::1',
'DOCUMENT_ROOT' => null,
'REQUEST_SCHEME' => 'http',
'REMOTE_PORT' => '80',
'REDIRECT_URL' => '/s/dashboard/',
'SERVER_PROTOCOL' => 'HTTP/1.1',
'REQUEST_METHOD' => 'GET',
'QUERY_STRING' => '',
'REQUEST_URI' => '/s/dashboard/',
'SCRIPT_NAME' => '/index.php',
'PHP_SELF' => '/index.php',
];

$request = new Request([], [], [], [], [], $server);

// google.com should not be returned as the URL
$this->assertEquals('https://test.com/s/dashboard', $this->getHelper()->getSafeRedirectUrl($request));
}

/**
* @return TrailingSlashHelper
*/
private function getHelper()
{
return new TrailingSlashHelper($this->coreParametersHelper);
}
}
Expand Up @@ -3,11 +3,11 @@
<?php if (count($errors) > 1): ?>
<ul>
<?php foreach ($errors as $error): ?>
<li><?php echo $error->getMessage() ?></li>
<li><?php echo $view->escape($error->getMessage()) ?></li>
<?php endforeach; ?>
</ul>
<?php else: ?>
<?php echo $errors[0]->getMessage() ?>
<?php echo $view->escape($errors[0]->getMessage()) ?>
<?php endif; ?>
</div>
<?php endif ?>
Expand Up @@ -10,11 +10,11 @@
<?php if (count($errorsMessages) > 1): ?>
<ul>
<?php foreach ($errorsMessages as $errorMessage): ?>
<li><?php echo $errorMessage; ?></li>
<li><?php echo $view->escape($errorMessage); ?></li>
<?php endforeach; ?>
</ul>
<?php else: ?>
<?php echo $errorsMessages[0]; ?>
<?php echo $view->escape($errorsMessages[0]); ?>
<?php endif; ?>
</div>
<?php endif; ?>
Expand Up @@ -13,6 +13,7 @@

use Doctrine\ORM\Tools\Pagination\Paginator;
use Mautic\CoreBundle\Entity\CommonRepository;
use Mautic\CoreBundle\Helper\Serializer;

/**
* DynamicContentRepository.
Expand Down Expand Up @@ -223,7 +224,7 @@ public function getDynamicContentForSlotFromCampaign($slot)
$result = $qb->execute()->fetchAll();

foreach ($result as $item) {
$properties = unserialize($item['properties']);
$properties = Serializer::decode($item['properties']);

if (isset($properties['dynamicContent'])) {
$dwc = $this->getEntity($properties['dynamicContent']);
Expand Down
3 changes: 2 additions & 1 deletion app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php
Expand Up @@ -12,6 +12,7 @@
namespace Mautic\EmailBundle\Command;

use Mautic\CoreBundle\Command\ModeratedCommand;
use Mautic\CoreBundle\Helper\Serializer;
use Mautic\EmailBundle\EmailEvents;
use Mautic\EmailBundle\Event\QueueEmailEvent;
use Symfony\Component\Console\Input\ArrayInput;
Expand Down Expand Up @@ -103,7 +104,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$tmpFilename .= '.finalretry';
rename($failedFile, $tmpFilename);

$message = unserialize(file_get_contents($tmpFilename));
$message = Serializer::decode(file_get_contents($tmpFilename), ['allowed_classes' => [\Swift_Message::class]]);
if ($message !== false && is_object($message) && get_class($message) === 'Swift_Message') {
$tryAgain = false;
if ($dispatcher->hasListeners(EmailEvents::EMAIL_RESEND)) {
Expand Down
3 changes: 2 additions & 1 deletion app/bundles/EmailBundle/DataFixtures/ORM/LoadEmailData.php
Expand Up @@ -15,6 +15,7 @@
use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
use Doctrine\Common\Persistence\ObjectManager;
use Mautic\CoreBundle\Helper\CsvHelper;
use Mautic\CoreBundle\Helper\Serializer;
use Mautic\EmailBundle\Entity\Email;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand Down Expand Up @@ -54,7 +55,7 @@ public function load(ObjectManager $manager)
if ($val != 'NULL') {
$setter = 'set'.ucfirst($col);
if (in_array($col, ['content', 'variantSettings'])) {
$val = unserialize(stripslashes($val));
$val = Serializer::decode(stripslashes($val));
}
$email->$setter($val);
}
Expand Down
5 changes: 3 additions & 2 deletions app/bundles/FormBundle/DataFixtures/ORM/LoadFormData.php
Expand Up @@ -15,6 +15,7 @@
use Doctrine\Common\DataFixtures\OrderedFixtureInterface;
use Doctrine\Common\Persistence\ObjectManager;
use Mautic\CoreBundle\Helper\CsvHelper;
use Mautic\CoreBundle\Helper\Serializer;
use Mautic\FormBundle\Entity\Action;
use Mautic\FormBundle\Entity\Field;
use Mautic\FormBundle\Entity\Form;
Expand Down Expand Up @@ -84,7 +85,7 @@ public function load(ObjectManager $manager)
$field->$setter($form);
$form->addField($count, $field);
} elseif (in_array($col, ['customParameters', 'properties'])) {
$val = unserialize(stripslashes($val));
$val = Serializer::decode(stripslashes($val));
$field->$setter($val);
} else {
$field->$setter($val);
Expand All @@ -106,7 +107,7 @@ public function load(ObjectManager $manager)
if (in_array($col, ['form'])) {
$action->$setter($this->getReference('form-'.$val));
} elseif (in_array($col, ['properties'])) {
$val = unserialize(stripslashes($val));
$val = Serializer::decode(stripslashes($val));
if ($col == 'settings') {
$val['callback'] = stripslashes($val['callback']);
}
Expand Down