diff --git a/app/bundles/CoreBundle/Helper/ClickthroughHelper.php b/app/bundles/CoreBundle/Helper/ClickthroughHelper.php index a4ec4481e53..15ac849bde3 100644 --- a/app/bundles/CoreBundle/Helper/ClickthroughHelper.php +++ b/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); diff --git a/app/bundles/CoreBundle/Helper/Serializer.php b/app/bundles/CoreBundle/Helper/Serializer.php index e258bde4abd..b7b3feb987a 100644 --- a/app/bundles/CoreBundle/Helper/Serializer.php +++ b/app/bundles/CoreBundle/Helper/Serializer.php @@ -19,6 +19,8 @@ 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 * @@ -26,6 +28,10 @@ class Serializer */ 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); } diff --git a/app/bundles/CoreBundle/Tests/functional/Entity/CommonRepositoryTest.php b/app/bundles/CoreBundle/Tests/Functional/Entity/CommonRepositoryTest.php similarity index 93% rename from app/bundles/CoreBundle/Tests/functional/Entity/CommonRepositoryTest.php rename to app/bundles/CoreBundle/Tests/Functional/Entity/CommonRepositoryTest.php index ac604b4c1c2..954307b9450 100644 --- a/app/bundles/CoreBundle/Tests/functional/Entity/CommonRepositoryTest.php +++ b/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; diff --git a/app/bundles/CoreBundle/Tests/functional/IpLookupFactoryCest.php b/app/bundles/CoreBundle/Tests/Functional/IpLookupFactoryCest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/functional/IpLookupFactoryCest.php rename to app/bundles/CoreBundle/Tests/Functional/IpLookupFactoryCest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Doctrine/ArrayTypeTest.php b/app/bundles/CoreBundle/Tests/Unit/Doctrine/ArrayTypeTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Doctrine/ArrayTypeTest.php rename to app/bundles/CoreBundle/Tests/Unit/Doctrine/ArrayTypeTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Entity/CommonRepositoryTest.php b/app/bundles/CoreBundle/Tests/Unit/Entity/CommonRepositoryTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Entity/CommonRepositoryTest.php rename to app/bundles/CoreBundle/Tests/Unit/Entity/CommonRepositoryTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Entity/IpAddressTest.php b/app/bundles/CoreBundle/Tests/Unit/Entity/IpAddressTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Entity/IpAddressTest.php rename to app/bundles/CoreBundle/Tests/Unit/Entity/IpAddressTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Event/CustomTemplateEventTest.php b/app/bundles/CoreBundle/Tests/Unit/Event/CustomTemplateEventTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Event/CustomTemplateEventTest.php rename to app/bundles/CoreBundle/Tests/Unit/Event/CustomTemplateEventTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/EventListener/CommonStatSubscriberTest.php b/app/bundles/CoreBundle/Tests/Unit/EventListener/CommonStatSubscriberTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/EventListener/CommonStatSubscriberTest.php rename to app/bundles/CoreBundle/Tests/Unit/EventListener/CommonStatSubscriberTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/EventListener/RequestSubscriberTest.php b/app/bundles/CoreBundle/Tests/Unit/EventListener/RequestSubscriberTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/EventListener/RequestSubscriberTest.php rename to app/bundles/CoreBundle/Tests/Unit/EventListener/RequestSubscriberTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Form/Validator/Constraints/CircularDependencyValidatorTest.php b/app/bundles/CoreBundle/Tests/Unit/Form/Validator/Constraints/CircularDependencyValidatorTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Form/Validator/Constraints/CircularDependencyValidatorTest.php rename to app/bundles/CoreBundle/Tests/Unit/Form/Validator/Constraints/CircularDependencyValidatorTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/AbstractFormFieldHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/AbstractFormFieldHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/AbstractFormFieldHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/AbstractFormFieldHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/ArrayHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/ArrayHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/ArrayHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/ArrayHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/Chart/LineChartTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/Chart/LineChartTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/Chart/LineChartTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/Chart/LineChartTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/ClickthroughHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/ClickthroughHelperTest.php similarity index 67% rename from app/bundles/CoreBundle/Tests/unit/Helper/ClickthroughHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/ClickthroughHelperTest.php index adff89b9f79..6018154220d 100644 --- a/app/bundles/CoreBundle/Tests/unit/Helper/ClickthroughHelperTest.php +++ b/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); diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/ColorHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/ColorHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/ColorHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/ColorHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/CsvHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/CsvHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/CsvHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/CsvHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/DateTimeHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/DateTimeHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/DateTimeHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/DateTimeHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/EncryptionHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/EncryptionHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/EncryptionHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/EncryptionHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/FileHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/FileHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/FileHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/FileHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/FilePathResolverTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/FilePathResolverTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/FilePathResolverTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/FilePathResolverTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/FileUploaderTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/FileUploaderTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/FileUploaderTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/FileUploaderTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/InputHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/InputHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/InputHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/InputHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/IpLookupHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/IpLookupHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/IpLookupHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/IpLookupHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/unit/Helper/RandomHelper/RandomHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/RandomHelper/RandomHelperTest.php similarity index 100% rename from app/bundles/CoreBundle/Tests/unit/Helper/RandomHelper/RandomHelperTest.php rename to app/bundles/CoreBundle/Tests/Unit/Helper/RandomHelper/RandomHelperTest.php diff --git a/app/bundles/CoreBundle/Tests/Unit/Helper/TestResources/WakeupCall.php b/app/bundles/CoreBundle/Tests/Unit/Helper/TestResources/WakeupCall.php new file mode 100644 index 00000000000..ae5c4bd5b3c --- /dev/null +++ b/app/bundles/CoreBundle/Tests/Unit/Helper/TestResources/WakeupCall.php @@ -0,0 +1,16 @@ + 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)) { diff --git a/app/bundles/UserBundle/Entity/User.php b/app/bundles/UserBundle/Entity/User.php index fb62d5e495e..31ebb7936de 100644 --- a/app/bundles/UserBundle/Entity/User.php +++ b/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); } diff --git a/app/bundles/UserBundle/Security/Authentication/Token/PluginToken.php b/app/bundles/UserBundle/Security/Authentication/Token/PluginToken.php index cd6605d83d6..aedb40e306b 100644 --- a/app/bundles/UserBundle/Security/Authentication/Token/PluginToken.php +++ b/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); } }