NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Security] Make sure RoleVoter only votes for RoleInterface objects and strings#19965

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

Gladhon wants to merge1 commit intosymfony:masterfromGladhon:master

Closed

[Security] Make sure RoleVoter only votes for RoleInterface objects and strings#19965

Gladhon wants to merge1 commit intosymfony:masterfromGladhon:master

Conversation

Copy link

Gladhon commentedSep 19, 2016•
edited
Loading

Q	A
Branch?	"master"
Bug fix?	yes
New feature?	no
BC breaks?	yes
Deprecations?	no
Tests pass?	yes
Fixed tickets	#18042
License	MIT
Doc PR

make sure RoleVoter only vote for roles and don't produce fatal error on non-strings.
The BC-Break happen, if someone use a Object as Role by not implementing the RoleInterface but adding a toString method which gives "ROLE_SOMETHING" back. This "feature" breaks.

carsonbot added Status: Needs Review Security Bug BC Break labels

Sep 19, 2016

Copy link

Contributor

sstok commentedSep 19, 2016

👍 for this change, but the CS needs some fixing. Please review the fabbot patch (once it's available).

Status: needs work

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Sep 19, 2016

[Security] make sure RoleVoter only vote for roles and don't produce …

3b88cb7

…fatal error on non-strings

Gladhon force-pushed themaster branch from5805705 to3b88cb7Compare

September 19, 2016 13:38

Copy link

Contributor

backbone87 commentedSep 21, 2016

obsoletes#19725 #19726

since this fixes the bug described in#18042, it should be backported to 2.x LTS (if any left)

Copy link

Author

Gladhon commentedSep 21, 2016

what's need to merge it in ?
@sstok cs should be fixed

javiereguiluz changed the title~~[Security] make sure RoleVoter only vote for roles and don't produce~~[Security] Make sure RoleVoter only votes for RoleInterface objects and strings

Sep 28, 2016

Copy link

Member

javiereguiluz commentedSep 28, 2016

👍

This looks like "the right thing to do" ... but we need to think carefully about the possible BC breaks.

Copy link

Contributor

backbone87 commentedSep 28, 2016

The only BC break was mentioned in the start post: When someone uses an object with __toString method without implementing the RoleInterface and using this object as an attribute to be checked by the role voter. I would consider this as a usage out of scope of the role voter API, because this voter is meant to operate on real strings or objects of type RoleInterface.

Copy link

Member

fabpot commentedOct 6, 2016

merged in 2.7 via#19725

fabpot closed this

Oct 6, 2016

Copy link

Author

Gladhon commentedOct 6, 2016•
edited
Loading

@fabpot great but it still not solved in the master branch

Copy link

Member

nicolas-grekas commentedOct 6, 2016•
edited
Loading

@Gladhon that's the process: bugs are fixed in the lowest branch where they apply, then these branches are merged up to master regularly by mergers. This could happen a few hours up to a few days later.