bug #30006 [Security] don't do nested calls to serialize() (nicolas-g…

…rekas, Renan) This PR was merged into the 3.4 branch. Discussion ---------- [Security] don't do nested calls to serialize() | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29951 | License | MIT | Doc PR | n/a The problem (originally reported as `Symfony\Component\Security\Core\Authentication\Token\AbstractToken` issue), may occur also in classes extending `Symfony\Component\Security\Core\Exception\AuthenticationException` Tasks: - [x] Skip native serializer (workaround itself) - [x] Token test - [x] Exception test Commits ------- 10256fc skip native serialize among child and parent serializable objects 41000f1 [Security] dont do nested calls to serialize()
symfony · Jan 29, 2019 · b4357d7 · b4357d7
2 parents 957b477 + 10256fc
commit b4357d7
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 43 deletions.
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
@@ -137,22 +137,17 @@ public function eraseCredentials()
      */
     public function serialize()
     {
-        return serialize(
-            [
-                \is_object($this->user) ? clone $this->user : $this->user,
-                $this->authenticated,
-                array_map(function ($role) { return clone $role; }, $this->roles),
-                $this->attributes,
-            ]
-        );
+        $serialized = [$this->user, $this->authenticated, $this->roles, $this->attributes];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($serialized)
     {
-        list($this->user, $this->authenticated, $this->roles, $this->attributes) = unserialize($serialized);
+        list($this->user, $this->authenticated, $this->roles, $this->attributes) = \is_array($serialized) ? $serialized : unserialize($serialized);
     }
 
     /**
@@ -232,6 +227,19 @@ public function __toString()
         return sprintf('%s(user="%s", authenticated=%s, roles="%s")', $class, $this->getUsername(), json_encode($this->authenticated), implode(', ', $roles));
     }
 
+    /**
+     * @internal
+     */
+    protected function doSerialize($serialized, $isCalledFromOverridingMethod)
+    {
+        if (null === $isCalledFromOverridingMethod) {
+            $trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 3);
+            $isCalledFromOverridingMethod = isset($trace[2]['function'], $trace[2]['object']) && 'serialize' === $trace[2]['function'] && $this === $trace[2]['object'];
+        }
+
+        return $isCalledFromOverridingMethod ? $serialized : serialize($serialized);
+    }
+
     private function hasUserChanged(UserInterface $user)
     {
         if (!($this->user instanceof UserInterface)) {

diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php
@@ -59,15 +59,17 @@ public function getSecret()
      */
     public function serialize()
     {
-        return serialize([$this->secret, parent::serialize()]);
+        $serialized = [$this->secret, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($serialized)
     {
-        list($this->secret, $parentStr) = unserialize($serialized);
+        list($this->secret, $parentStr) = \is_array($serialized) ? $serialized : unserialize($serialized);
         parent::unserialize($parentStr);
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php
@@ -79,15 +79,17 @@ public function eraseCredentials()
      */
     public function serialize()
     {
-        return serialize([$this->credentials, $this->providerKey, parent::serialize()]);
+        $serialized = [$this->credentials, $this->providerKey, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($str)
     {
-        list($this->credentials, $this->providerKey, $parentStr) = unserialize($str);
+        list($this->credentials, $this->providerKey, $parentStr) = \is_array($str) ? $str : unserialize($str);
         parent::unserialize($parentStr);
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php
@@ -94,19 +94,17 @@ public function getCredentials()
      */
     public function serialize()
     {
-        return serialize([
-            $this->secret,
-            $this->providerKey,
-            parent::serialize(),
-        ]);
+        $serialized = [$this->secret, $this->providerKey, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($serialized)
     {
-        list($this->secret, $this->providerKey, $parentStr) = unserialize($serialized);
+        list($this->secret, $this->providerKey, $parentStr) = \is_array($serialized) ? $serialized : unserialize($serialized);
         parent::unserialize($parentStr);
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php
@@ -91,15 +91,17 @@ public function eraseCredentials()
      */
     public function serialize()
     {
-        return serialize([$this->credentials, $this->providerKey, parent::serialize()]);
+        $serialized = [$this->credentials, $this->providerKey, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($serialized)
     {
-        list($this->credentials, $this->providerKey, $parentStr) = unserialize($serialized);
+        list($this->credentials, $this->providerKey, $parentStr) = \is_array($serialized) ? $serialized : unserialize($serialized);
         parent::unserialize($parentStr);
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Exception/AccountStatusException.php b/src/Symfony/Component/Security/Core/Exception/AccountStatusException.php
@@ -44,18 +44,17 @@ public function setUser(UserInterface $user)
      */
     public function serialize()
     {
-        return serialize([
-            $this->user,
-            parent::serialize(),
-        ]);
+        $serialized = [$this->user, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($str)
     {
-        list($this->user, $parentData) = unserialize($str);
+        list($this->user, $parentData) = \is_array($str) ? $str : unserialize($str);
 
         parent::unserialize($parentData);
     }

diff --git a/src/Symfony/Component/Security/Core/Exception/AuthenticationException.php b/src/Symfony/Component/Security/Core/Exception/AuthenticationException.php
@@ -38,15 +38,33 @@ public function setToken(TokenInterface $token)
         $this->token = $token;
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function serialize()
     {
-        return serialize([
+        $serialized = [
             $this->token,
             $this->code,
             $this->message,
             $this->file,
             $this->line,
-        ]);
+        ];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
+    }
+
+    /**
+     * @internal
+     */
+    protected function doSerialize($serialized, $isCalledFromOverridingMethod)
+    {
+        if (null === $isCalledFromOverridingMethod) {
+            $trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 3);
+            $isCalledFromOverridingMethod = isset($trace[2]['function'], $trace[2]['object']) && 'serialize' === $trace[2]['function'] && $this === $trace[2]['object'];
+        }
+
+        return $isCalledFromOverridingMethod ? $serialized : serialize($serialized);
     }
 
     public function unserialize($str)
@@ -57,7 +75,7 @@ public function unserialize($str)
             $this->message,
             $this->file,
             $this->line
-        ) = unserialize($str);
+        ) = \is_array($str) ? $str : unserialize($str);
     }
 
     /**

diff --git a/src/Symfony/Component/Security/Core/Exception/CustomUserMessageAuthenticationException.php b/src/Symfony/Component/Security/Core/Exception/CustomUserMessageAuthenticationException.php
@@ -60,19 +60,17 @@ public function getMessageData()
      */
     public function serialize()
     {
-        return serialize([
-            parent::serialize(),
-            $this->messageKey,
-            $this->messageData,
-        ]);
+        return serialize([parent::serialize(true), $this->messageKey, $this->messageData]);
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($str)
     {
-        list($parentData, $this->messageKey, $this->messageData) = unserialize($str);
+        list($parentData, $this->messageKey, $this->messageData) = \is_array($str) ? $str : unserialize($str);
 
         parent::unserialize($parentData);
     }

diff --git a/src/Symfony/Component/Security/Core/Exception/UsernameNotFoundException.php b/src/Symfony/Component/Security/Core/Exception/UsernameNotFoundException.php
@@ -54,18 +54,17 @@ public function setUsername($username)
      */
     public function serialize()
     {
-        return serialize([
-            $this->username,
-            parent::serialize(),
-        ]);
+        $serialized = [$this->username, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($str)
     {
-        list($this->username, $parentData) = unserialize($str);
+        list($this->username, $parentData) = \is_array($str) ? $str : unserialize($str);
 
         parent::unserialize($parentData);
     }

diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
@@ -43,9 +43,14 @@ public function __construct($user, array $roles = [])
         $this->setUser($user);
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function serialize()
     {
-        return serialize([$this->credentials, parent::serialize()]);
+        $serialized = [$this->credentials, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     public function unserialize($serialized)

diff --git a/.../Component/Security/Core/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php b/.../Component/Security/Core/Tests/Exception/CustomUserMessageAuthenticationExceptionTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Core\Tests\Exception;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
 use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException;
 
 class CustomUserMessageAuthenticationExceptionTest extends TestCase
@@ -24,4 +25,18 @@ public function testConstructWithSAfeMessage()
         $this->assertEquals(['foo' => true], $e->getMessageData());
         $this->assertEquals('SAFE MESSAGE', $e->getMessage());
     }
+
+    public function testSharedSerializedData()
+    {
+        $token = new AnonymousToken('foo', 'bar');
+
+        $exception = new CustomUserMessageAuthenticationException();
+        $exception->setToken($token);
+        $exception->setSafeMessage('message', ['token' => $token]);
+
+        $processed = unserialize(serialize($exception));
+        $this->assertEquals($token, $processed->getToken());
+        $this->assertEquals($token, $processed->getMessageData()['token']);
+        $this->assertSame($processed->getToken(), $processed->getMessageData()['token']);
+    }
 }
diff --git a/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php b/src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php
@@ -76,15 +76,17 @@ public function getProviderKey()
      */
     public function serialize()
     {
-        return serialize([$this->providerKey, parent::serialize()]);
+        $serialized = [$this->providerKey, parent::serialize(true)];
+
+        return $this->doSerialize($serialized, \func_num_args() ? \func_get_arg(0) : null);
     }
 
     /**
      * {@inheritdoc}
      */
     public function unserialize($serialized)
     {
-        list($this->providerKey, $parentStr) = unserialize($serialized);
+        list($this->providerKey, $parentStr) = \is_array($serialized) ? $serialized : unserialize($serialized);
         parent::unserialize($parentStr);
     }
 }