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

See http://www.silverstripe.org/ss-2013-005-privilege-escalation-through-apply-roles-assignment/

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

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);
+	}
 }

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);
+	}
 }

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'
+		);
+	}
 }