Permalink
Browse files

Merging from HEAD:

Fixing all incoming parameters when editing course groups as per SC#171.
  • Loading branch information...
1 parent b7535a6 commit 8b68aaf1f6e1447fb910fdcda2d9c3c97644b461 defacer committed Oct 26, 2005
Showing with 24 additions and 6 deletions.
  1. +14 −5 course/group.php
  2. +10 −1 course/groups.php
View
@@ -51,11 +51,20 @@
$group->picture = 1;
}
}
- $group->name = clean_text($form->name);
- $group->description = clean_text($form->description);
- $group->hidepicture = $form->hidepicture;
- $group->password = $form->password;
- if (!update_record("groups", $group)) {
+
+ // Setting a new object in order to avoid updating other columns for the record,
+ // which could lead to SQL injection vulnerabilities.
+
+ // Be VERY sure to sanitize all parameters that go into $dataobj!
+
+ $dataobj = new stdClass;
+ $dataobj->id = $group->id;
+ $dataobj->name = clean_text($form->name);
+ $dataobj->description = clean_text($form->description);
+ $dataobj->hidepicture = empty($form->hidepicture) ? 0 : 1;
+ $dataobj->password = required_param('password', PARAM_ALPHANUM);
+
+ if (!update_record('groups', $dataobj)) {
notify("A strange error occurred while trying to save");
} else {
notify(get_string('changessaved'));
View
@@ -47,6 +47,12 @@
if ($data = data_submitted() and confirm_sesskey()) {
+ // Clean ALL incoming parameters which go in SQL queries here for good measure
+ $data->id = required_param('id', PARAM_INT);
+ $data->groups = optional_param('groups', 0, PARAM_INT);
+ $data->groupid = optional_param('groupid', 0, PARAM_INT);
+ $data->members = optional_param('members', array(), PARAM_INT);
+
if (!empty($data->nonmembersadd)) { /// Add people to a group
if (!empty($data->nonmembers) and !empty($data->groupid)) {
$groupmodified = false;
@@ -76,6 +82,9 @@
} else if (!empty($data->groupsremove)) { /// Remove a group, all members become nonmembers
if (!empty($data->groups)) {
+ if(!isset($groups[$data->groups])) {
+ error("This is not a valid group to remove");
+ }
delete_records("groups", "id", $data->groups);
delete_records("groups_members", "groupid", $data->groups);
unset($groups[$data->groups]);
@@ -102,8 +111,8 @@
if (!empty($data->members) and !empty($data->groupid)) {
foreach ($data->members as $userid) {
delete_records('groups_members', 'userid', $userid, "groupid", $data->groupid);
- set_field('groups', 'timemodified', time(), 'id', $data->groupid);
}
+ set_field('groups', 'timemodified', time(), 'id', $data->groupid);
}
$selectedgroup = $data->groupid;

0 comments on commit 8b68aaf

Please sign in to comment.