Browse files

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

  • Loading branch information...
1 parent 46556b6 commit cfa88adf4b3b7fad5a509d1512df8d0ca808a3b6 @chillu chillu committed Aug 30, 2013
View
5 docs/en/changelogs/3.0.6.md
@@ -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
16 security/PermissionRole.php
@@ -76,4 +76,20 @@ public function fieldLabels($includerelations = true) {
return $labels;
}
+
+ public function canView($member = null) {
+ return Permission::check('APPLY_ROLES', 'any', $member);
+ }
+
+ 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
34 security/PermissionRoleCode.php
@@ -13,4 +13,38 @@ class PermissionRoleCode extends DataObject {
private 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
31 tests/security/PermissionRoleTest.php
@@ -16,4 +16,35 @@ public function testDelete() {
$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'
+ );
+ }
}

0 comments on commit cfa88ad

Please sign in to comment.