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

Groups doctrine deserialization #812

Closed

Conversation

bertuz
Copy link

@bertuz bertuz commented Aug 19, 2017

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License Apache-2.0

Doctrine's ObjectConstructor ignored the Context groups that can be specified when deserializing. An identifier could be sent and it was used to find an entity, irrespective of its group. This is not a problem if groups are not used while deserializing, but can cause big security concerns if groups are used to not let the user send data that could affect persisted data.
The pull request adds some tests to show it and it tries to fix it without altering the other functionalities.

Matteo Bertamini added 12 commits August 18, 2017 18:41
tests did not performed correctly because of an empty classMetadata
…ding to the other tests.

Some refactoring of the tests should be performed too (move something into setup method + removing duplicating setup among other my test methods)
if an identifier is passed but is excluded by the group rules, the fallback objectconstructor is called (doctrine can't find the entity without the id, which must be ignored in this case)
…class for backward compatibility reasons (final class in old versions)
@vwatel
Copy link

vwatel commented Aug 20, 2020

Hi @goetas

Do you have any news on this pull request?
According to Veracode, this is the possible security patch for an existing vulnerability within the library.

Thank you.

@goetas
Copy link
Collaborator

goetas commented Aug 20, 2020

Hi, Unfortunately this pr is not implemented in a way that makes it ready to be merged. It works only for annotations (not for yaml or xml metadata). Do you want to resume the work on it?

@goetas
Copy link
Collaborator

goetas commented Aug 22, 2020

Implemented in #1246

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

Successfully merging this pull request may close these issues.

3 participants