Merge pull request #7755 from mautic/release-2.15.2

Release 2.15.2 to staging
mautic · Aug 2, 2019 · af5e056 · af5e056
2 parents 30909dc + 7302a31
commit af5e056
Show file tree

Hide file tree

Showing 41 changed files with 1,685 additions and 1,358 deletions.
diff --git a/.htaccess b/.htaccess
@@ -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>
diff --git a/app/bundles/CampaignBundle/Views/Campaign/leads.html.php b/app/bundles/CampaignBundle/Views/Campaign/leads.html.php
@@ -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; ?>" />

diff --git a/app/bundles/CoreBundle/Assets/js/libraries/3a.chosen.jquery.js b/app/bundles/CoreBundle/Assets/js/libraries/3a.chosen.jquery.js
diff --git a/app/bundles/CoreBundle/Config/config.php b/app/bundles/CoreBundle/Config/config.php
@@ -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' => [

diff --git a/app/bundles/CoreBundle/Controller/CommonController.php b/app/bundles/CoreBundle/Controller/CommonController.php
@@ -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;
@@ -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);
     }
 
     /**

diff --git a/app/bundles/CoreBundle/Helper/ClickthroughHelper.php b/app/bundles/CoreBundle/Helper/ClickthroughHelper.php
@@ -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);
     }
 }
diff --git a/app/bundles/CoreBundle/Helper/EncryptionHelper.php b/app/bundles/CoreBundle/Helper/EncryptionHelper.php
@@ -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;

diff --git a/app/bundles/CoreBundle/Helper/Serializer.php b/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);
+    }
+}
diff --git a/app/bundles/CoreBundle/Helper/TrailingSlashHelper.php b/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;
+    }
+}
diff --git a/app/bundles/CoreBundle/Templating/Helper/FormatterHelper.php b/app/bundles/CoreBundle/Templating/Helper/FormatterHelper.php
@@ -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;
 
@@ -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;
                     }

diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/TrailingSlashHelperTest.php b/app/bundles/CoreBundle/Tests/unit/Helper/TrailingSlashHelperTest.php
@@ -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);
+    }
+}
diff --git a/app/bundles/CoreBundle/Views/FormTheme/Custom/form_errors.html.php b/app/bundles/CoreBundle/Views/FormTheme/Custom/form_errors.html.php
@@ -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 ?>
diff --git a/app/bundles/CoreBundle/Views/FormTheme/Custom/sortablelist_errors.html.php b/app/bundles/CoreBundle/Views/FormTheme/Custom/sortablelist_errors.html.php
@@ -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; ?>
diff --git a/app/bundles/DynamicContentBundle/Entity/DynamicContentRepository.php b/app/bundles/DynamicContentBundle/Entity/DynamicContentRepository.php
@@ -13,6 +13,7 @@
 
 use Doctrine\ORM\Tools\Pagination\Paginator;
 use Mautic\CoreBundle\Entity\CommonRepository;
+use Mautic\CoreBundle\Helper\Serializer;
 
 /**
  * DynamicContentRepository.
@@ -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']);

diff --git a/app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php b/app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php
@@ -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;
@@ -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)) {

diff --git a/app/bundles/EmailBundle/DataFixtures/ORM/LoadEmailData.php b/app/bundles/EmailBundle/DataFixtures/ORM/LoadEmailData.php
@@ -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;
@@ -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);
                 }

diff --git a/app/bundles/FormBundle/DataFixtures/ORM/LoadFormData.php b/app/bundles/FormBundle/DataFixtures/ORM/LoadFormData.php
@@ -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;
@@ -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);
@@ -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']);
                         }