FIX Privilege escalation through APPLY_ROLES assignment (SS-2013-005)

See http://www.silverstripe.org/ss-2013-005-privilege-escalation-through-apply-roles-assignment/
This commit is contained in:
Ingo Schommer 2013-08-30 13:59:38 +02:00
parent 797951595b
commit ec8e8261f2
3 changed files with 78 additions and 1 deletions

View File

@ -62,4 +62,16 @@ class PermissionRole extends DataObject {
$code->delete(); $code->delete();
} }
} }
public function canCreate($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
public function canEdit($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
public function canDelete($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
} }

View File

@ -13,4 +13,38 @@ class PermissionRoleCode extends DataObject {
static $has_one = array( static $has_one = array(
"Role" => "PermissionRole", "Role" => "PermissionRole",
); );
protected function validate() {
$result = parent::validate();
// Check that new code doesn't increase privileges, unless an admin is editing.
$privilegedCodes = Permission::$privileged_permissions;
if(
$this->Code
&& in_array($this->Code, $privilegedCodes)
&& !Permission::check('ADMIN')
) {
$result->error(sprintf(
_t(
'PermissionRoleCode.PermsError',
'Can\'t assign code "%s" with privileged permissions (requires ADMIN access)'
),
$this->Code
));
}
return $result;
}
public function canCreate($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
public function canEdit($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
public function canDelete($member = null) {
return Permission::check('APPLY_ROLES', 'any', $member);
}
} }

View File

@ -14,4 +14,35 @@ class PermissionRoleTest extends FunctionalTest {
$this->assertNull(DataObject::get('PermissionRole', "\"ID\"={$role->ID}"), 'Role is removed'); $this->assertNull(DataObject::get('PermissionRole', "\"ID\"={$role->ID}"), 'Role is removed');
$this->assertNull(DataObject::get('PermissionRoleCode',"\"RoleID\"={$role->ID}"), 'Permissions removed along with the role'); $this->assertNull(DataObject::get('PermissionRoleCode',"\"RoleID\"={$role->ID}"), 'Permissions removed along with the role');
} }
public function testValidatesPrivilegedPermissions() {
$nonAdminCode = new PermissionRoleCode(array('Code' => 'CMS_ACCESS_CMSMain'));
$nonAdminValidateMethod = new ReflectionMethod($nonAdminCode, 'validate');
$nonAdminValidateMethod->setAccessible(true);
$adminCode = new PermissionRoleCode(array('Code' => 'ADMIN'));
$adminValidateMethod = new ReflectionMethod($adminCode, 'validate');
$adminValidateMethod->setAccessible(true);
$this->logInWithPermission('APPLY_ROLES');
$result = $nonAdminValidateMethod->invoke($nonAdminCode);
$this->assertTrue(
$result->valid(),
'Members with only APPLY_ROLES can create non-privileged permission role codes'
);
$this->logInWithPermission('APPLY_ROLES');
$result = $adminValidateMethod->invoke($adminCode);
$this->assertFalse(
$result->valid(),
'Members with only APPLY_ROLES can\'t create privileged permission role codes'
);
$this->logInWithPermission('ADMIN');
$result = $adminValidateMethod->invoke($adminCode);
$this->assertTrue(
$result->valid(),
'Members with ADMIN can create privileged permission role codes'
);
}
} }