Skip to content

Commit

Permalink
security #cve-2019-18889 [Cache] forbid serializing AbstractAdapter a…
Browse files Browse the repository at this point in the history
…nd TagAwareAdapter instances (nicolas-grekas)

This PR was merged into the 3.4 branch.
  • Loading branch information
nicolas-grekas committed Nov 12, 2019
2 parents b21025b + 1507413 commit 4cc37df
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,16 @@ public function commit()
return $ok;
}

public function __sleep()
{
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
}

public function __wakeup()
{
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
}

public function __destruct()
{
if ($this->deferred) {
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/Cache/Adapter/TagAwareAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,16 @@ public function commit()
return $this->invalidateTags([]);
}

public function __sleep()
{
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);

This comment has been minimized.

Copy link
@rishabhgpt

rishabhgpt Feb 20, 2020

My application is throwing this exception, using Tags as caching strategy. Can some one help ?

This comment has been minimized.

Copy link
@stof

stof Feb 20, 2020

Member

please open an issue (with reproducing steps and a stack trace) rather than commenting on a commit. Comments on commits can only be found through notifications (or by looking at each commit, but that's not something anyone does), and that's easy to miss (and it won't let us go back to older discussions, or track the fact a bug is not fixed) and is not visible to people not watching the repo.

}

public function __wakeup()
{
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
}

public function __destruct()
{
$this->commit();
Expand Down

4 comments on commit 4cc37df

@NicoleG25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nicolas-grekas , could you explain to me please why AbstractAdapter.php is fixed only in the 3.4.x branch and TagAwareAdapter.php is fixed in the others as well?
I would like to understand what is the file that is vulnerable and if its AbstractAdapter.php and the fix isn't applied to the other branches then doesn't that mean they're still vulnerable?
Thanks in advance ! :)

@xabbuh
Copy link
Member

@xabbuh xabbuh commented on 4cc37df Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always merge branches up into all other still maintained branches which means that changes made in 3.4 will be available in more recent versions too.

@NicoleG25
Copy link

@NicoleG25 NicoleG25 commented on 4cc37df Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always merge branches up into all other still maintained branches which means that changes made in 3.4 will be available in more recent versions too.
@xabbuh

I understand but that being said, the fix applied to AbstractAdapter.php does not appear in more recent branches at all..
Like for example, the fix doesn't appear in 4.3.x as far as I could tell.

@nicolas-grekas
Copy link
Member Author

@nicolas-grekas nicolas-grekas commented on 4cc37df Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On upper branches, AbstractAdapter has been refactored, the patch applies to AbstractAdapterTrait instead:
https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Cache/Traits/AbstractAdapterTrait.php

Please sign in to comment.