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

[Security] Complain about an empty decision strategy #29981

Merged
merged 1 commit into from Feb 21, 2019

Conversation

@corphi
Copy link
Contributor

corphi commented Jan 24, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

When an empty string is passed (or objects with a similarly behaving __toString() method) to the constructor, the call to decide causes infinite recursion.

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 44fb1ff to 35d3cc8 Jan 24, 2019

@corphi

This comment has been minimized.

Copy link
Contributor Author

corphi commented Jan 26, 2019

The actual issue is of course that the method names are not prefix-free.

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 35d3cc8 to ab12ca8 Jan 26, 2019

@xabbuh
Copy link
Member

xabbuh left a comment

I think we should rather check that the passed value is not an empty string. The proposed solution has the drawback that it will fail again in the future if we introduce a doDecide() method for whatever reason.

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from ab12ca8 to c537c85 Jan 27, 2019

@corphi corphi changed the base branch from master to 3.4 Jan 27, 2019

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from c537c85 to 42e34f4 Jan 27, 2019

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 42e34f4 to 88502a5 Jan 28, 2019

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jan 30, 2019

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 88502a5 to 228329a Feb 3, 2019

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 228329a to 21f73d2 Feb 11, 2019

@corphi corphi force-pushed the corphi:fix-infinite-recursion branch from 21f73d2 to 3a22cad Feb 19, 2019

@corphi

This comment has been minimized.

Copy link
Contributor Author

corphi commented Feb 19, 2019

I applied the requested changes; the label didn’t change, though.

@xabbuh

xabbuh approved these changes Feb 20, 2019

@fabpot

fabpot approved these changes Feb 21, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Feb 21, 2019

Thank you @corphi.

@fabpot fabpot merged commit 3a22cad into symfony:3.4 Feb 21, 2019

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Feb 21, 2019

bug #29981 [Security] Complain about an empty decision strategy (corphi)
This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Complain about an empty decision strategy

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

When an empty string is passed (or objects with a similarly behaving `__toString()` method) to the constructor, the call to `decide` causes infinite recursion.

Commits
-------

3a22cad Fix infinite recursion when passed an empty string

@corphi corphi deleted the corphi:fix-infinite-recursion branch Feb 21, 2019

This was referenced Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.