Skip to content
Browse files

security report MDL-20834 Added a check for moodle/backup:userinfo to…

… inform the admin about people who might be able to backup user data
  • Loading branch information...
1 parent deacd77 commit 47ee4d187f9d07320ead3c5b0b03f17fe452d271 @moodler moodler committed Nov 25, 2009
Showing with 87 additions and 1 deletion.
  1. +76 −0 admin/report/security/lib.php
  2. +11 −1 lang/en_utf8/report_security.php
View
76 admin/report/security/lib.php
@@ -59,6 +59,7 @@ function report_security_get_issue_list() {
'report_security_check_configrw',
'report_security_check_riskxss',
'report_security_check_riskadmin',
+ 'report_security_check_riskbackup',
'report_security_check_defaultuserrole',
'report_security_check_guestrole',
'report_security_check_frontpagerole',
@@ -1092,3 +1093,78 @@ function report_security_check_riskadmin($detailed=false) {
return $result;
}
+
+/**
+ * Lists all roles that have the ability to backup user data, as well as users
+ * @param bool $detailed
+ * @return object result
+ */
+function report_security_check_riskbackup($detailed=false) {
+ global $CFG;
+
+ $result = new object();
+ $result->issue = 'report_security_check_riskbackup';
+ $result->name = get_string('check_riskbackup_name', 'report_security');
+ $result->info = null;
+ $result->details = null;
+ $result->status = null;
+ $result->link = null;
+
+ if ($roles = get_roles_with_capability('moodle/backup:userinfo', CAP_ALLOW)) {
+ // Find all the users who have the potential ability to backup user info. Ignoring the actual backup capability for now
+ // because it could easily be ALLOWed later and then these user info caps would suddenly be a problem.
+ $sqluserinfo = "SELECT u.id, u.firstname, u.lastname, u.picture, u.imagealt, u.email, ra.contextid, ra.roleid
+ FROM (SELECT rcx.*
+ FROM {$CFG->prefix}role_capabilities rcx
+ WHERE rcx.capability = 'moodle/backup:userinfo' AND rcx.permission = ".CAP_ALLOW.") rc,
+ {$CFG->prefix}context c,
+ {$CFG->prefix}context sc,
+ {$CFG->prefix}role_assignments ra,
+ {$CFG->prefix}user u
+ WHERE c.id = rc.contextid
+ AND (sc.path = c.path OR sc.path LIKE ".sql_concat('c.path', "'/%'")." OR c.path LIKE ".sql_concat('sc.path', "'/%'").")
+ AND u.id = ra.userid AND u.deleted = 0
+ AND ra.contextid = sc.id AND ra.roleid = rc.roleid AND sc.contextlevel <= ".CONTEXT_COURSE."
+ GROUP BY u.id, u.firstname, u.lastname, u.picture, u.imagealt, u.email, ra.contextid, ra.roleid
+ ORDER BY u.lastname, u.firstname";
+
+ $usercount = count_records_sql("SELECT COUNT('x') FROM ($sqluserinfo) userinfo");
+
+ $result->status = REPORT_SECURITY_WARNING;
+ $a = (object)array('rolecount'=>count($roles), 'usercount'=>$usercount);
+ $result->info = get_string('check_riskbackup_warning', 'report_security', $a);
+
+ if ($detailed) {
+ // Make a list of roles
+ foreach ($roles as $role) {
+ $role->url = "$CFG->wwwroot/$CFG->admin/roles/manage.php?action=edit&amp;roleid=$role->id";
+ $rolelinks[] = '<li>'.get_string('check_riskbackup_editrole', 'report_security', $role).'</li>';
+ }
+ $rolelinks = '<ul>'.implode($rolelinks).'</ul>';
+
+ // Get a list of users as well
+ $rs = get_recordset_sql($sqluserinfo);
+ $users = array();
+ while ($user = rs_fetch_next_record($rs)) {
+ $url = "$CFG->wwwroot/$CFG->admin/roles/assign.php?contextid=$user->contextid&amp;roleid=$user->roleid";
+ $a = (object)array('fullname'=>fullname($user), 'url'=>$url, 'email'=>$user->email);
+ $users[] = '<li>'.get_string('check_riskbackup_unassign', 'report_security', $a).'</li>';
+ }
+ rs_close($rs);
+ $users = '<ul>'.implode($users).'</ul>';
+
+ // Combine as one big list of info
+ $a = (object)array('roles'=>$rolelinks, 'users'=>$users);
+ $result->details = get_string('check_riskbackup_detailswarning', 'report_security', $a);
+ }
+ } else {
+ $result->status = REPORT_SECURITY_OK;
+ $result->info = get_string('check_riskbackup_ok', 'report_security');
+
+ if ($detailed) {
+ $result->details = get_string('check_riskbackup_detailsok', 'report_security');
+ }
+ }
+
+ return $result;
+}
View
12 lang/en_utf8/report_security.php
@@ -130,11 +130,21 @@
<p>Please refer to the <a href=\"$a\" target=\"_blank\">password salting documentation</a> if you wish to change the password salt. Once set, do NOT delete your password salt otherwise you will no longer be able to login to your site!</p>';
$string['check_riskadmin_detailsok'] = '<p>Please verify the following list of system administrators:</p>$a';
$string['check_riskadmin_detailswarning'] = '<p>Please verify the following list of system administrators:</p>$a->admins
-<p>It is recommended to assign administrator role in system context only. Following users have unsupported admin role assignments:</p>$a->unsupported';
+<p>It is recommended to assign administrator role in the system context only. The following users have (unsupported) admin role assignments in other contexts:</p>$a->unsupported';
$string['check_riskadmin_name'] = 'Administrators';
$string['check_riskadmin_ok'] = 'Found $a server administrator(s).';
$string['check_riskadmin_unassign'] = '<a href=\"$a->url\">$a->fullname ($a->email) review role assignment</a>';
$string['check_riskadmin_warning'] = 'Found $a->admincount server administrators and $a->unsupcount unsupported admin role assignments.';
+$string['check_riskadmin_name'] = 'Administrators';
+
+$string['check_riskbackup_name'] = 'Backup of user data';
+$string['check_riskbackup_warning'] = 'Found $a->rolecount roles and $a->usercount users with the ability to backup user data.';
+$string['check_riskbackup_detailswarning'] = '<p>Please verify the following roles should allow the backup of user data:</p>$a->roles
+<p>These user accounts currently have the capability to make backups containing user data from the whole site. Make sure they are (a) trusted and (b) protected by strong passwords:</p>$a->users';
+$string['check_riskbackup_editrole'] = '<a href=\"$a->url\">$a->name</a>';
+$string['check_riskbackup_unassign'] = '<a href=\"$a->url\">$a->fullname ($a->email)</a>';
+$string['check_riskbackup_ok'] = 'No roles explicitly allow backup of user data';
+$string['check_riskbackup_detailsok'] = 'No roles explicitly allow backup of user data. However, note that admins with the \"doanything\" capability are still likely to be able to do this.';
$string['check_riskxss_details'] = '<p>RISK_XSS denotes all dangerous capabilities that only trusted users may use.</p>
<p>Please verify the following list of users and make sure that you trust them completely on this server:</p><p>$a</p>';

0 comments on commit 47ee4d1

Please sign in to comment.
Something went wrong with that request. Please try again.