Merge pull request #7929 from mautic/release-2.15.3

Post release 2.15.3

app/bundles/CoreBundle/Helper/ClickthroughHelper.php

@@ -42,8 +42,8 @@ public static function decodeArrayFromUrl($string, $urlDecode = true)
             return [];
         }
 
-        if (strpos(strtolower($decoded), 'a') !== 0) {
-            throw new \InvalidArgumentException(sprintf('The string %s is not a serialized array.', $decoded));
+        if (stripos($decoded, 'a') !== 0) {
+            throw new \InvalidArgumentException(sprintf('The string %s is not a serialized array', $decoded));
         }
 
         return Serializer::decode($decoded);

app/bundles/CoreBundle/Helper/Serializer.php

@@ -19,13 +19,19 @@ class Serializer
      * 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.
      *
+     * PHP does not recommend untrusted user input even with ['allowed_classes' => false]
+     *
      * @param string $serializedString
      * @param array  $options
      *
      * @return mixed
      */
     public static function decode($serializedString, array $options = ['allowed_classes' => false])
     {
+        if (stripos($serializedString, 'o:') !== false) {
+            throw new \InvalidArgumentException(sprintf('The string %s contains an object.', $serializedString));
+        }
+
         if (version_compare(phpversion(), '7.0.0', '<')) {
             return unserialize($serializedString);
         }

app/bundles/CoreBundle/Tests/functional/Entity/CommonRepositoryTest.php → app/bundles/CoreBundle/Tests/Functional/Entity/CommonRepositoryTest.php

@@ -9,7 +9,7 @@
  * @license     GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
  */
 
-namespace Mautic\CoreBundle\Tests\functional\Entity;
+namespace Mautic\CoreBundle\Tests\Functional\Entity;
 
 use Mautic\CoreBundle\Test\MauticMysqlTestCase;
 

app/bundles/CoreBundle/Tests/unit/Helper/ClickthroughHelperTest.php → app/bundles/CoreBundle/Tests/Unit/Helper/ClickthroughHelperTest.php

@@ -9,9 +9,10 @@
  * @license     GNU/GPLv3 http://www.gnu.org/licenses/gpl-3.0.html
  */
 
-namespace Mautic\CoreBundle\Tests\Helper;
+namespace Mautic\CoreBundle\Tests\Unit\Helper;
 
 use Mautic\CoreBundle\Helper\ClickthroughHelper;
+use Mautic\CoreBundle\Tests\Unit\Helper\TestResources\WakeupCall;
 
 class ClickthroughHelperTest extends \PHPUnit_Framework_TestCase
 {
@@ -22,6 +23,18 @@ public function testEncodingCanBeDecoded()
         $this->assertEquals($array, ClickthroughHelper::decodeArrayFromUrl(ClickthroughHelper::encodeArrayForUrl($array)));
     }
 
+    /**
+     * @covers \Mautic\CoreBundle\Helper\Serializer::decode
+     */
+    public function testObjectInArrayIsDetectedOrIgnored()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+
+        $array = ['foo' => new WakeupCall()];
+
+        ClickthroughHelper::decodeArrayFromUrl(ClickthroughHelper::encodeArrayForUrl($array));
+    }
+
     public function testOnlyArraysCanBeDecodedToPreventObjectWakeupVulnerability()
     {
         $this->expectException(\InvalidArgumentException::class);

app/bundles/CoreBundle/Tests/Unit/Helper/TestResources/WakeupCall.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace Mautic\CoreBundle\Tests\Unit\Helper\TestResources;
+
+class WakeupCall
+{
+    public function __wakeup()
+    {
+        throw new \Exception('this should not have been executed');
+    }
+
+    public function hello()
+    {
+        return 'test';
+    }
+}

app/bundles/EmailBundle/Command/ProcessEmailQueueCommand.php

@@ -12,7 +12,6 @@
 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 +102,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
                     $tmpFilename .= '.finalretry';
                     rename($failedFile, $tmpFilename);
 
-                    $message = Serializer::decode(file_get_contents($tmpFilename), ['allowed_classes' => true]);
+                    $message = unserialize(file_get_contents($tmpFilename));
                     if ($message !== false && is_object($message) && get_class($message) === 'Swift_Message') {
                         $tryAgain = false;
                         if ($dispatcher->hasListeners(EmailEvents::EMAIL_RESEND)) {

app/bundles/UserBundle/Entity/User.php

@@ -15,7 +15,6 @@
 use Mautic\ApiBundle\Serializer\Driver\ApiMetadataDriver;
 use Mautic\CoreBundle\Doctrine\Mapping\ClassMetadataBuilder;
 use Mautic\CoreBundle\Entity\FormEntity;
-use Mautic\CoreBundle\Helper\Serializer;
 use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
 use Symfony\Component\Form\Form;
 use Symfony\Component\Security\Core\User\AdvancedUserInterface;
@@ -467,7 +466,7 @@ public function unserialize($serialized)
             $this->username,
             $this->password,
             $published
-            ) = Serializer::decode($serialized);
+            ) = unserialize($serialized);
         $this->setIsPublished($published);
     }
 

app/bundles/UserBundle/Security/Authentication/Token/PluginToken.php

@@ -11,7 +11,6 @@
 
 namespace Mautic\UserBundle\Security\Authentication\Token;
 
-use Mautic\CoreBundle\Helper\Serializer;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
 
@@ -113,7 +112,7 @@ public function serialize()
      */
     public function unserialize($serialized)
     {
-        list($this->authenticatingService, $this->credentials, $this->providerKey, $parentStr) = Serializer::decode($serialized);
+        list($this->authenticatingService, $this->credentials, $this->providerKey, $parentStr) = unserialize($serialized);
         parent::unserialize($parentStr);
     }
 }