mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 12:05:37 +00:00
FIX Privilege escalation through Group hierarchy setting (SS-2013-003)
See http://www.silverstripe.org/ss-2013-003-privilege-escalation-through-group-hierarchy-setting/
This commit is contained in:
parent
84a8b21936
commit
797951595b
15
docs/en/changelogs/2.4.12.md
Normal file
15
docs/en/changelogs/2.4.12.md
Normal file
@ -0,0 +1,15 @@
|
||||
# 2.4.12
|
||||
|
||||
## Overview
|
||||
|
||||
### 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/)
|
||||
|
||||
### Security: Privilege escalation through Group and Member CSV upload (SS-2013-004)
|
||||
|
||||
See [announcement](http://www.silverstripe.org/ss-2013-004-privilege-escalation-through-group-and-member-csv-upload/)
|
||||
|
||||
### Security: Privilege escalation through APPLY_ROLES assignment (SS-2013-005)
|
||||
|
||||
See [announcement](http://www.silverstripe.org/ss-2013-005-privilege-escalation-through-apply-roles-assignment/)
|
@ -319,6 +319,33 @@ class Group extends DataObject {
|
||||
$this->setField("Code",SiteTree::generateURLSegment($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->ParentID && !Permission::check('ADMIN')) {
|
||||
$ancestorIds = $this->Parent()->collateAncestorIDs();
|
||||
$inheritedPermissions = DataObject::get(
|
||||
'Permission',
|
||||
sprintf('"GroupID" IN (%s)', implode(',', array_map('intval', $ancestorIds)))
|
||||
);
|
||||
$inheritedCodes = $inheritedPermissions ? $inheritedPermissions->column('Code') : array();
|
||||
if(array_intersect($inheritedCodes, Permission::$privileged_permissions)) {
|
||||
$result->error(sprintf(
|
||||
_t(
|
||||
'Group.HierarchyPermsError',
|
||||
'Can\'t assign parent group "%s" with privileged permissions (requires ADMIN access)'
|
||||
),
|
||||
$this->Parent()->Title
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
function onBeforeWrite() {
|
||||
parent::onBeforeWrite();
|
||||
|
||||
|
@ -90,6 +90,17 @@ class Permission extends DataObject {
|
||||
*/
|
||||
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.
|
||||
*
|
||||
|
@ -86,6 +86,54 @@ class GroupTest extends FunctionalTest {
|
||||
$this->assertNull(DataObject::get('Group', "\"ID\"={$adminGroup->ID}"), 'Group is removed');
|
||||
$this->assertNull(DataObject::get('Permission',"\"GroupID\"={$adminGroup->ID}"), 'Permissions removed along with the group');
|
||||
}
|
||||
|
||||
public function testValidatesPrivilegeLevelOfParent() {
|
||||
if(!class_exists('ReflectionMethod')) {
|
||||
$this->markTestSkipped('Test requires PHP 5.3 Reflection API');
|
||||
}
|
||||
|
||||
$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 {
|
||||
|
Loading…
x
Reference in New Issue
Block a user