Permalink
Browse files

MDL-24081 completion Fixing sql injections and use of sql_ilike()

  • Loading branch information...
1 parent e6c2edb commit 5642a8e51586df7a129b14904ea4f558fd704e9d @srynot4sale srynot4sale committed Sep 8, 2010
Showing with 68 additions and 34 deletions.
  1. +8 −5 course/report/completion/index.php
  2. +8 −5 course/report/progress/index.php
  3. +52 −24 lib/completionlib.php
View
13 course/report/completion/index.php
@@ -184,21 +184,23 @@ function csv_quote($value) {
// Generate where clause
$where = array();
-$ilike = $DB->sql_ilike();
+$where_params = array();
if ($sifirst !== 'all') {
- $where[] = "u.firstname $ilike '$sifirst%'";
+ $where[] = $DB->sql_like('u.firstname', ':sifirst', false);
+ $where_params['sifirst'] = $sifirst.'%';
}
if ($silast !== 'all') {
- $where[] = "u.lastname $ilike '$silast%'";
+ $where[] = $DB->sql_like('u.lastname', ':silast', false);
+ $where_params['silast'] = $silast.'%';
}
// Get user match count
-$total = $completion->get_num_tracked_users(implode(' AND ', $where), $group);
+$total = $completion->get_num_tracked_users(implode(' AND ', $where), $where_params, $group);
// Total user count
-$grandtotal = $completion->get_num_tracked_users('', $group);
+$grandtotal = $completion->get_num_tracked_users('', array(), $group);
// If no users in this course what-so-ever
if (!$grandtotal) {
@@ -213,6 +215,7 @@ function csv_quote($value) {
if ($total) {
$progress = $completion->get_progress_all(
implode(' AND ', $where),
+ $where_params,
$group,
$firstnamesort ? 'u.firstname ASC' : 'u.lastname ASC',
$csv ? 0 : COMPLETION_REPORT_PAGE,
View
13 course/report/progress/index.php
@@ -78,21 +78,23 @@ function csv_quote($value) {
// Generate where clause
$where = array();
-$ilike = $DB->sql_ilike();
+$where_params = array();
if ($sifirst !== 'all') {
- $where[] = "u.firstname $ilike '$sifirst%'";
+ $where[] = $DB->sql_like('u.firstname', ':sifirst', false);
+ $where_params['sifirst'] = $sifirst.'%';
}
if ($silast !== 'all') {
- $where[] = "u.lastname $ilike '$silast%'";
+ $where[] = $DB->sql_like('u.lastname', ':silast', false);
+ $where_params['silast'] = $silast.'%';
}
// Get user match count
-$total = $completion->get_num_tracked_users(implode(' AND ', $where), $group);
+$total = $completion->get_num_tracked_users(implode(' AND ', $where), $where_params, $group);
// Total user count
-$grandtotal = $completion->get_num_tracked_users('', $group);
+$grandtotal = $completion->get_num_tracked_users('', array(), $group);
// If no users in this course what-so-ever
if (!$grandtotal) {
@@ -110,6 +112,7 @@ function csv_quote($value) {
if ($total) {
$progress = $completion->get_progress_all(
implode(' AND ', $where),
+ $where_params,
$group,
$firstnamesort ? 'u.firstname ASC' : 'u.lastname ASC',
$csv ? 0 : COMPLETION_REPORT_PAGE,
View
76 lib/completionlib.php
@@ -973,10 +973,15 @@ public function get_activities($modinfo=null) {
function is_tracked_user($userid) {
global $DB;
+ $tracked = $this->generate_tracked_user_sql();
+
$sql = "SELECT u.id ";
- $sql .= $this->generate_tracked_user_sql();
+ $sql .= $tracked->sql;
$sql .= ' AND u.id = :user';
- return $DB->record_exists_sql($sql, array('user' => (int)$userid));
+
+ $params = $tracked->data;
+ $params['user'] = (int)$userid;
+ return $DB->record_exists_sql($sql, $params);
}
@@ -985,21 +990,25 @@ function is_tracked_user($userid) {
*
* Optionally supply a search's where clause, or a group id
*
- * @param string $where Where clause
+ * @param string $where Where clause sql
+ * @param array $where_params Where clause params
* @param int $groupid Group id
* @return int
*/
- function get_num_tracked_users($where = '', $groupid = 0) {
+ function get_num_tracked_users($where = '', $where_params = array(), $groupid = 0) {
global $DB;
+ $tracked = $this->generate_tracked_user_sql($groupid);
+
$sql = "SELECT COUNT(u.id) ";
- $sql .= $this->generate_tracked_user_sql($groupid);
+ $sql .= $tracked->sql;
if ($where) {
$sql .= " AND $where";
}
- return $DB->count_records_sql($sql);
+ $params = array_merge($tracked->data, $where_params);
+ return $DB->count_records_sql($sql, $params);
}
@@ -1008,18 +1017,22 @@ function get_num_tracked_users($where = '', $groupid = 0) {
*
* Optionally supply a search's where caluse, group id, sorting, paging
*
- * @param string $where Where clause (optional)
+ * @param string $where Where clause sql (optional)
+ * @param array $where_params Where clause params (optional)
* @param integer $groupid Group ID to restrict to (optional)
* @param string $sort Order by clause (optional)
* @param integer $limitfrom Result start (optional)
* @param integer $limitnum Result max size (optional)
* @return array
*/
- function get_tracked_users($where = '', $groupid = 0, $sort = '',
- $limitfrom = '', $limitnum = '') {
+ function get_tracked_users($where = '', $where_params = array(), $groupid = 0,
+ $sort = '', $limitfrom = '', $limitnum = '') {
global $DB;
+ $tracked = $this->generate_tracked_user_sql($groupid);
+ $params = $tracked->data;
+
$sql = "
SELECT
u.id,
@@ -1028,32 +1041,38 @@ function get_tracked_users($where = '', $groupid = 0, $sort = '',
u.idnumber
";
- $sql .= $this->generate_tracked_user_sql($groupid);
+ $sql .= $tracked->sql;
if ($where) {
$sql .= " AND $where";
+ $params = array_merge($params, $where_params);
}
if ($sort) {
$sql .= " ORDER BY $sort";
}
- $users = $DB->get_records_sql($sql, null, $limitfrom, $limitnum);
+ $users = $DB->get_records_sql($sql, $params, $limitfrom, $limitnum);
return $users ? $users : array(); // In case it returns false
}
/**
* Generate the SQL for finding tracked users in this course
*
- * Optionally supply a group id
+ * Returns an object containing the sql fragment and an array of
+ * bound data params.
*
* @param integer $groupid
- * @return string
+ * @return object
*/
function generate_tracked_user_sql($groupid = 0) {
global $CFG;
+ $return = new stdClass();
+ $return->sql = '';
+ $return->data = array();
+
if (!empty($CFG->progresstrackedroles)) {
$roles = ' AND ra.roleid IN ('.$CFG->progresstrackedroles.')';
} else {
@@ -1074,12 +1093,12 @@ function generate_tracked_user_sql($groupid = 0) {
if ($groupid) {
$groupjoin = "JOIN {groups_members} gm
ON gm.userid = u.id";
- $groupselect = " AND gm.groupid = $groupid ";
+ $groupselect = " AND gm.groupid = :groupid ";
+
+ $return->data['groupid'] = $groupid;
}
- $now = time();
-
- $sql = "
+ $return->sql = "
FROM
{user} u
INNER JOIN
@@ -1099,17 +1118,23 @@ function generate_tracked_user_sql($groupid = 0) {
ON c.id = e.courseid
$groupjoin
WHERE
- (ra.contextid = {$context->id} $parentcontexts)
- AND c.id = {$this->course->id}
+ (ra.contextid = :contextid $parentcontexts)
+ AND c.id = :courseid
AND ue.status = 0
AND e.status = 0
- AND ue.timestart < $now
- AND (ue.timeend > $now OR ue.timeend = 0)
+ AND ue.timestart < :now1
+ AND (ue.timeend > :now2 OR ue.timeend = 0)
$groupselect
$roles
";
- return $sql;
+ $now = time();
+ $return->data['now1'] = $now;
+ $return->data['now2'] = $now;
+ $return->data['contextid'] = $context->id;
+ $return->data['courseid'] = $this->course->id;
+
+ return $return;
}
/**
@@ -1126,18 +1151,21 @@ function generate_tracked_user_sql($groupid = 0) {
* @global object
* @param bool $sortfirstname If true, sort by first name, otherwise sort by
* last name
+ * @param string $where Where clause sql (optional)
+ * @param array $where_params Where clause params (optional)
* @param int $groupid Group ID or 0 (default)/false for all groups
* @param int $pagesize Number of users to actually return (optional)
* @param int $start User to start at if paging (optional)
* @return Object with ->total and ->start (same as $start) and ->users;
* an array of user objects (like mdl_user id, firstname, lastname)
* containing an additional ->progress array of coursemoduleid => completionstate
*/
- public function get_progress_all($where = '', $groupid = 0, $sort = '', $pagesize = '', $start = '') {
+ public function get_progress_all($where = '', $where_params = array(), $groupid = 0,
+ $sort = '', $pagesize = '', $start = '') {
global $CFG, $DB;
// Get list of applicable users
- $users = $this->get_tracked_users($where, $groupid, $sort, $start, $pagesize);
+ $users = $this->get_tracked_users($where, $where_params, $groupid, $sort, $start, $pagesize);
// Get progress information for these users in groups of 1, 000 (if needed)
// to avoid making the SQL IN too long

0 comments on commit 5642a8e

Please sign in to comment.