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

Possible issue with unserialize in PHP 7.3 #29459

Closed
yellow1912 opened this issue Dec 5, 2018 · 40 comments
Closed

Possible issue with unserialize in PHP 7.3 #29459

yellow1912 opened this issue Dec 5, 2018 · 40 comments
Labels
Milestone

Comments

@yellow1912
Copy link

yellow1912 commented Dec 5, 2018

Symfony version(s) affected: 3.4.19

Description

Upon upgrading to PHP 7.3, we run into issue with the unserialization of the user/token object when login.

The error is like this:

Notice: unserialize(): Error at offset 5859 of 13936 byte

The exact error happens in:
in vendor/symfony/symfony/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php (line 155)

How to reproduce

It seems like this is due to the serialized object contains object class inside it.

Not sure what changed in PHP 7.3 RC6, at first I thought it could be the unserialize now requires allowed_classes option to be set to true explicitly, I tried that by editing Symfony code but it didn't help

Possible Solution

Not a clue for now

Additional context

Everything was running fine on PHP 7.2 so it must be the changes in 7.3 RC6 that break things.

I attached here a sample of the string to unserialize:
https://gist.github.com/yellow1912/43e4009e0384426ccfa017feb0eedcc9

@broncha
Copy link

broncha commented Dec 8, 2018

For me, I just had my User entity implement \Serializable and limited what gets serialized, so its just an array. If you were using FOSUserBundle you would not have this problem

@javiereguiluz javiereguiluz added this to the 3.4 milestone Dec 9, 2018
@andrewmy
Copy link
Contributor

andrewmy commented Dec 9, 2018

Reproduced this in 4.1.7

@adrienbrignon
Copy link

Having the same issue on PHP 7.3.0.

@chalasr
Copy link
Member

chalasr commented Dec 10, 2018

Could someone please provide a minimal app with enough code to reproduce?

@javiereguiluz
Copy link
Member

javiereguiluz commented Dec 11, 2018

I'm facing the exact same issue with 4.1.* and 4.2.* when using PHP 7.2.

@marc-arnoult
Copy link

Hi, same issue for me with docker. ( php7.3.0-fpm )

Notice: unserialize(): Error at offset 1383 of 2467 bytes
in vendor/symfony/security-guard/Token/PostAuthenticationGuardToken.php->unserialize (line 88)

@yellow1912
Copy link
Author

For me, I just had my User entity implement \Serializable and limited what gets serialized, so its just an array. If you were using FOSUserBundle you would not have this problem

@broncha yes surely it can be a temporary fix (and perhaps best practice), but this is still a bug. If object is not supported, then exception should be thrown when we try to store an object inside.

@yellow1912
Copy link
Author

I'm facing the exact same issue with 4.1.* and 4.2.* when using PHP 7.2.

@javiereguiluz 7.2 with 3.4.20 works for me, perhaps things are different on 4.x. For now with 3.4.x I only see breaks in php 7.3

@javiereguiluz
Copy link
Member

I've debugged this a bit more. First: this is NOT a PHP 7.3 issue. I don't have PHP 7.3 on my machine and I suffered this issue when upgrading Symfony.

The most recent Symfony version that works for me is 4.1.6. All the following versions show the same error: 4.1.7, 4.1.8, 4.1.9, 4.2.0 and 4.2.1.

@broncha
Copy link

broncha commented Dec 12, 2018

@yellow1912 I agree. At least this is not blocking anything anymore for me.

@xabbuh
Copy link
Member

xabbuh commented Dec 12, 2018

Could anyone provide an example application that allows to reproduce?

@xabbuh
Copy link
Member

xabbuh commented Dec 13, 2018

Some guessing: Are you all using the entity user provider? And did you by any chance also update Doctrine related packages?

@broncha
Copy link

broncha commented Dec 13, 2018

@xabbuh i was using custom provider. Not the stock entity provider. I might have updated doctrine packages.

@yellow1912
Copy link
Author

For me I was using FOS but then later dropped it and now also using our own user provider. For doctrine, I think we updated it along with SF as well.

@analogic
Copy link

@javiereguiluz it seem like PHP problem, see https://3v4l.org/gTumn

It is serialized user at 7.3.0 at symfony 3.4.20, custom authenticator from SimpleFormAuthenticatorInterface

@nicolas-grekas
Copy link
Member

Looks like a php 7.3 bug, did anyone report it to get feedback from the PHP team?

@javiereguiluz
Copy link
Member

@nicolas-grekas I'm not sure. I don't have PHP 7.3 on my machine (I have PHP 7.2.7) and I can see this error with some Symfony versions but not others:

  • It works: Symfony 4.1.6 and all previous versions.
  • It fails: Symfony 4.1.7, 4.1.8, 4.1.9, 4.2.0 and 4.2.1.

@chalasr
Copy link
Member

chalasr commented Dec 15, 2018

@javiereguiluz Could you give the steps to reproduce? Does it happen on symfony-demo?

@javiereguiluz
Copy link
Member

@chalasr it's a private app ... and it's complex ... I tried to extract a reproducer but it's not possible to me :(

I'm going to send you via email the full error page in case you see something in the error trace. Thanks!

@analogic
Copy link

@nicolas-grekas there was no similar issue at bug tracker, so I've reported it https://bugs.php.net/bug.php?id=77302

@nicolas-grekas
Copy link
Member

I reproduced the issue on an app, there is definitely a PHP bug:
serializing a Serializable object with other Serializable objects inside can lead to broken internal references. Here is a broken serialized string for one of these nested serializables:
a:3:{i:0;s:86:"https://connect.sensiolabs.com/api/users/072fe4aa-78a1-45c7-b802-34eb632562ef/projects";i:1;N;i:2;a:7:{s:5:"items";a:3:{i:0;r:1203;i:1;r:1220;i:2;r:1237;}s:5:"total";s:1:"3";s:5:"count";s:1:"3";s:5:"index";s:1:"0";s:5:"limit";s:1:"3";s:7:"nextUrl";N;s:7:"prevUrl";N;}}

r:1203 there makes no sense.

We hit the bug because of this line added in 4.1.7.

@javiereguiluz
Copy link
Member

I confirm that reverting the changes made in #28072 fixed the problem for me. Thanks Nicolas!

@chalasr
Copy link
Member

chalasr commented Dec 15, 2018

See #29621 for the fix

nicolas-grekas added a commit that referenced this issue Dec 17, 2018
…r user refreshment (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Prefer clone() over unserialize(serialize()) for user refreshment

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29459
| License       | MIT
| Doc PR        | n/a

To not hit the `serialize()` bug reported in the related ticket

Commits
-------

a8eba80 [Security] Prefer clone over unserialize(serialize()) for user refreshment
@analogic
Copy link

Another place where php bug is hit

/**
* {@inheritdoc}
*/
public function serialize()
{
return serialize(array($this->credentials, $this->providerKey, parent::serialize()));
}
/**
* {@inheritdoc}
*/
public function unserialize($serialized)
{
list($this->credentials, $this->providerKey, $parentStr) = unserialize($serialized);
parent::unserialize($parentStr);
}

@analogic
Copy link

I would bet that there will be other places which are using serialize/unserialize and where can end up embedded objects.

@nicolas-grekas
Copy link
Member

@analogic did you actually hit the bug in other places? Can you provide some stack trace/reproducer?

@broncha
Copy link

broncha commented Dec 17, 2018

I had it yesterday in a Datacollector. Forgot to take detail

@analogic
Copy link

@nicolas-grekas not yet, UsernamePasswordToken.php only (fix not works for me). Imho there is not much what can be done at symfony, I will adjust my user structure...

@BenchmarkingBuffalo
Copy link

BenchmarkingBuffalo commented Jan 7, 2019

Hi there,
I am new to developing with Symfony, but I get the same error. The funny thing is, that on the first page I load after logging in as an user everything works. If I load any other page after that. I get an error from the unserialize function.
grafik
I have a entity structure, where one user is connected to one restaurant and one restaurant can have many dishes. Each dish is connected to many days. My configuration is:
grafik
I just used the instruction given here: https://symfony.com/doc/current/security/form_login_setup.html
And I added to the security.yaml:

providers:
our_db_provider:
entity:
class: App\Entity\User
property: username

I hope, this helps to solve the problem. If there are more questions arising, I wil be glad to answer them.

@BenchmarkingBuffalo
Copy link

Ok, it seems to be a problem arising from using php v7.3.0. When I use v7.1.25, everything works fine. But still it would be awesome if there would be a work around so noone runs into this php-based error:)

@nicolas-grekas
Copy link
Member

That's a PHP bug, it should be reported on http://bugs.php.net/ with a simple reproducer.
I'd be awesome if you could provide that!

@BenchmarkingBuffalo
Copy link

Ok, I will try to build a simple application asap:)

@yellow1912
Copy link
Author

If you report PHP bug, I think you can add the example to this one:

https://bugs.php.net/bug.php?id=77302

It will help the php developers to confirm the existence of this known bug.

@toooni
Copy link
Contributor

toooni commented Jan 22, 2019

The same issue exist in PostAuthenticationGuardToken::serialize because the token is serialized twice.
I tried to make a PR for a workaround but I am stuck doing this without BC break. We can't overwrite serialize and unserialize from AbstractToken because the properties are private.
Any idea how to proceed there? I'll gladly make a PR if someone points me in the right direction.

@jdrzewinski
Copy link

@toooni found same issue, silenced error in logs :

unserialize(): Error at offset 2560 of 4524 bytes {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\SilencedErrorContext: {\"severity\":8,\"file\":\"/var/www/symfony/vendor/symfony/security-core/Authentication/Token/AbstractToken.php\",\"line\":154,\"trace\":[{\"file\":\"/var/www/symfony/vendor/symfony/security-guard/Token/PostAuthenticationGuardToken.php\",\"line\":88,\"function\":\"unserialize\",\"class\":\"Symfony\\\\Component\\\\Security\\\\Core\\\\Authentication\\\\Token\\\\AbstractToken\",\"type\":\"->\"}],\"count\":1})"}

@oleg-superbody
Copy link

From PHP bug discussion:

[2019-01-22 19:57 UTC] dmitry@php.net

7.2 and below are affected by the same problem.
The old versions don't fail, but silently produce incorrect result (not the same as was serialized).

[2019-01-22 20:23 UTC] nikic@php.net

I plan to propose and implement a new custom object serialization mechanism for PHP 7.4, to replace the Serializable interface and all the problems that come with it.

For now, all I can suggest is to rewrite your code in a way that does not use parent::serialize(). I don't think there is anything we can do to fix Serializable itself, unfortunately.

So I bet it will not be fixed until php 7.4 and we have to use some workaround.

@yoannk
Copy link

yoannk commented Jan 26, 2019

Same issue with php 7.3 and symfony 4.2, with php 7.2.14 all works fine.

@nicolas-grekas
Copy link
Member

See #29951

@rajeshr101
Copy link

I am trying to run my application But, I cant run becuase, I am getting this error as - Notice: unserialize(): Error at offset 909 of 2109 bytes.
what to do to resolve this issue.Is there any solution available to resolve this problem?
I am using PHP 7.3
isssue1

@xabbuh
Copy link
Member

xabbuh commented Jun 8, 2020

Symfony 2.8 isn't maintained anymore since a long time. You should update to a supported version first and check if that already resolves your issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests