Fixed #3454. #3479
Fixed #3454. #3479
Conversation
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.
|
I've just updated this PR with a fix. |
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.
|
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 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? |
|
@jacobkiers I'd suggest backporting the fix for the escalation issue to master via another PR. We can merge both at the same time. |
|
@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. |
ghost
assigned
|
@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 😄 |
|
Definitely 2.1.0 -- most likely we'll also cut a 2.0.7 at the time we On Fri, Jan 18, 2013 at 4:11 PM, Jacob Kiers notifications@github.comwrote:
Matthew Weier O'Phinney |
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 😄