tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

FIX Privilege escalation through Group hierarchy setting (SS-2013-003)

Browse Source 
See http://www.silverstripe.org/ss-2013-003-privilege-escalation-through-group-hierarchy-setting/
This commit is contained in:
Ingo Schommer 2013-08-30 13:58:37 +02:00
parent 62608a7772
commit 720c149aee
4 changed files with 83 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
docs/en/changelogs/3.0.6.md
View File

@ -7,7 +7,7 @@
## Details
### Security: Require ADMIN for ?flush=1
### Security: Require ADMIN for ?flush=1 (SS-2013-001)
Flushing the various manifests (class, template, config) is performed through a GET
parameter (`flush=1`). Since this action requires more server resources than normal requests,
@ -23,6 +23,9 @@ This applies to both `flush=1` and `flush=all` (technically we only check for th
but only through web requests made through main.php - CLI requests, or any other request that goes through
a custom start up script will still process all flush requests as normal.
### Security: Privilege escalation through Group hierarchy setting (SS-2013-003)
See [announcement](http://www.silverstripe.org/ss-2013-003-privilege-escalation-through-group-hierarchy-setting/)
## Upgrading
* If you have created your own composite database fields, then you should amend the setValue() to allow the passing of

25
security/Group.php
View File

@ -345,6 +345,31 @@ class Group extends DataObject {
$this->setField("Code", Convert::raw2url($val));
}
public function validate() {
$result = parent::validate();
// Check if the new group hierarchy would add certain "privileged permissions",
// and require an admin to perform this change in case it does.
// This prevents "sub-admin" users with group editing permissions to increase their privileges.
if($this->Parent()->exists() && !Permission::check('ADMIN')) {
$inheritedCodes = Permission::get()
->filter('GroupID', $this->Parent()->collateAncestorIDs())
->column('Code');
$privilegedCodes = Config::inst()->get('Permission', 'privileged_permissions');
if(array_intersect($inheritedCodes, $privilegedCodes)) {
$result->error(sprintf(
_t(
'Group.HierarchyPermsError',
'Can\'t assign parent group "%s" with privileged permissions (requires ADMIN access)'
),
$this->Parent()->Title
));
}
}
return $result;
}
public function onBeforeWrite() {
parent::onBeforeWrite();

11
security/Permission.php
View File

@ -90,6 +90,17 @@ class Permission extends DataObject implements TemplateGlobalProvider {
*/
static $hidden_permissions = array();
/**
* @config These permissions can only be applied by ADMIN users, to prevent
* privilege escalation on group assignments and inheritance.
* @var array
*/
static $privileged_permissions = array(
'ADMIN',
'APPLY_ROLES',
'EDIT_PERMISSIONS'
);
/**
* Check that the current member has the given permission.
*

43
tests/security/GroupTest.php
View File

@ -133,6 +133,49 @@ class GroupTest extends FunctionalTest {
'Grandchild groups are removed');
}
public function testValidatesPrivilegeLevelOfParent() {
$nonAdminUser = $this->objFromFixture('GroupTest_Member', 'childgroupuser');
$adminUser = $this->objFromFixture('GroupTest_Member', 'admin');
$nonAdminGroup = $this->objFromFixture('Group', 'childgroup');
$adminGroup = $this->objFromFixture('Group', 'admingroup');
$nonAdminValidateMethod = new ReflectionMethod($nonAdminGroup, 'validate');
$nonAdminValidateMethod->setAccessible(true);
// Making admin group parent of a non-admin group, effectively expanding is privileges
$nonAdminGroup->ParentID = $adminGroup->ID;
$this->logInWithPermission('APPLY_ROLES');
$result = $nonAdminValidateMethod->invoke($nonAdminGroup);
$this->assertFalse(
$result->valid(),
'Members with only APPLY_ROLES can\'t assign parent groups with direct ADMIN permissions'
);
$this->logInWithPermission('ADMIN');
$result = $nonAdminValidateMethod->invoke($nonAdminGroup);
$this->assertTrue(
$result->valid(),
'Members with ADMIN can assign parent groups with direct ADMIN permissions'
);
$nonAdminGroup->write();
$newlyAdminGroup = $nonAdminGroup;
$this->logInWithPermission('ADMIN');
$inheritedAdminGroup = $this->objFromFixture('Group', 'group1');
$inheritedAdminMethod = new ReflectionMethod($inheritedAdminGroup, 'validate');
$inheritedAdminMethod->setAccessible(true);
$inheritedAdminGroup->ParentID = $adminGroup->ID;
$inheritedAdminGroup->write(); // only works with ADMIN login
$this->logInWithPermission('APPLY_ROLES');
$result = $inheritedAdminMethod->invoke($nonAdminGroup);
$this->assertFalse(
$result->valid(),
'Members with only APPLY_ROLES can\'t assign parent groups with inherited ADMIN permission'
);
}
}
class GroupTest_Member extends Member implements TestOnly {