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 6cff9671d4
commit 05757efceb
4 changed files with 82 additions and 0 deletions

View File

@ -30,6 +30,11 @@ See [announcement](http://www.silverstripe.org/ss-2013-003-privilege-escalation-
### 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/)
## Upgrading
* If you have created your own composite database fields, then you should amend the setValue() to allow the passing of

View File

@ -74,4 +74,16 @@ class PermissionRole extends DataObject {
return $labels;
}
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(
"Role" => "PermissionRole",
);
protected function validate() {
$result = parent::validate();
// Check that new code doesn't increase privileges, unless an admin is editing.
$privilegedCodes = Config::inst()->get('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

@ -16,4 +16,35 @@ class PermissionRoleTest extends FunctionalTest {
$this->assertEquals(0, DataObject::get('PermissionRoleCode',"\"RoleID\"={$role->ID}")->count(),
'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'
);
}
}