Fixed #3454. #3479

Closed
wants to merge 4 commits into
from

3 participants

@jacobkiers

As stated in the my comment in #3454, access to a resource is denied with a global deny policy and a permission on a parent.

This PR just adds a test to show the behaviour. I might be able to write a fix, but I'm not yet sure of that. The ACL code is quite complex 😄

jacobkiers added some commits Jan 18, 2013
@jacobkiers jacobkiers Added test for #3454. 07e4245
@jacobkiers jacobkiers Fix #3454.
The fix involves forcing a loop through the parent resources when the
current resource is 'TYPE_DENIED'. The loop stops when we are already at
the top-level resource.
3c88e7b
@jacobkiers

I've just updated this PR with a fix.

@jacobkiers jacobkiers Ensure privilege escalation is not possible.
With the previous version, a deny() on a lower-lever resource was not
inherited when on a higher-lever resource an allow() was given.

See the updated test for more an example.
58352d1
@jacobkiers

I found that some privilege escaliation is possible when inheritance does not work corectly. I will check somewhere tonight if this is due to my previous changes, or that it also happens in the master branch.

For now, could someone please review these changes? Thanks!

@Maks3w

$return will only have the last $child

@jacobkiers

@Maks3w I've tested this on the master branch (just the tests), and found that the privilege escalation issue is there too.

What do you propose? Backporting this one or sending in a new PR with just the changes needed to fix the security issue?

@weierophinney
Zend Framework member

@jacobkiers I'd suggest backporting the fix for the escalation issue to master via another PR. We can merge both at the same time.

@weierophinney
Zend Framework member

@jacobkiers Actually, if you don't mind, I'll simply cherry-pick it into master. This will rewrite history, so you'll need to do a 'reset --hard upstream/master` (as well as upstream/develop) after merged.

@weierophinney weierophinney added a commit that referenced this pull request Jan 18, 2013
@weierophinney weierophinney Merge branch 'hotfix/3479' into develop
Forward port #3479
5f60ea1
@weierophinney weierophinney added a commit that closed this pull request Jan 18, 2013
@weierophinney weierophinney Merge branch 'hotfix/3479'
Close #3479
Fixes #3454
c0e2d53
@jacobkiers

@weierophinney I don't mind at all. That means these fixes will make it into 2.0.7, right? That would make me really happy 😄

@jacobkiers jacobkiers deleted the jacobkiers:zf3454 branch Jan 18, 2013
@weierophinney
Zend Framework member
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/3479'
Close #3479
Fixes #3454
0217b59
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/3479' into develop
Forward port #3479
2141c93
@weierophinney weierophinney added a commit to zendframework/zend-permissions-acl that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/3479' df5b9c3
@weierophinney weierophinney added a commit to zendframework/zend-permissions-acl that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/3479' into develop d32c68c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment