Add safe(r) query abstraction

Add db_query_safe that accepts parameters separately. Use it in
some places.

Author: nikic

include/cvs-auth.inc

@@ -12,26 +12,18 @@ function verify_password($user, $pass)
 {
     db_connect();
 
-    $username = real_clean($user);
-    $res = mysql_query("SELECT svnpasswd FROM users WHERE cvsaccess AND username = '$username'");
+    $res = db_query_safe("SELECT svnpasswd FROM users WHERE cvsaccess AND username = ?", [$user]);
 
     if ($res && mysql_num_rows($res) == 1) {
         $row = mysql_fetch_array($res);
-        return gen_svn_pass($username, $pass) === $row["svnpasswd"];
+        return gen_svn_pass($user, $pass) === $row["svnpasswd"];
     }
 
     return false;
 }
 
 function verify_username($user) {
     db_connect();
-
-    $username = real_clean($user);
-    $res = mysql_query("SELECT 1 FROM users WHERE cvsaccess AND username = '$username'");
-
-    if ($res && mysql_num_rows($res) == 1) {
-        return true;
-    }
-
-    return false;
+    $res = db_query_safe("SELECT 1 FROM users WHERE cvsaccess AND username = ?", [$user]);
+    return $res && mysql_num_rows($res) == 1;
 }

include/functions.inc

@@ -119,6 +119,21 @@ function db_query($query)
     return $res;
 }
 
+function db_prepare_query($query, $params) {
+    if (substr_count($query, '?') !== count($params)) {
+        die("Incorrect number of parameters to query.");
+    }
+
+    $i = 0;
+    return preg_replace_callback('/\?/', function() use ($params, &$i) {
+        return "'" . mysql_real_escape_string($params[$i++]) . "'";
+    }, $query);
+}
+
+function db_query_safe($query, array $params = [])
+{
+    return db_query(db_prepare_query($query, $params));
+}
 
 function db_get_one($query)
 {
@@ -198,7 +213,7 @@ function show_prev_next($begin, $rows, $skip, $total, $extra = [], $table = true
 
 function show_country_options($cc = "")
 {
-    $res = db_query("SELECT id, name FROM country ORDER BY name");
+    $res = db_query_safe("SELECT id, name FROM country ORDER BY name");
     while ($row = mysql_fetch_assoc($res)) {
         echo "<option value=\"{$row['id']}\"", $cc == $row['id'] ? " selected" : "", ">{$row['name']}</option>";
     }
@@ -291,7 +306,7 @@ function undo_magic_quotes() {
 
 
 function find_group_address_from_notes_for($id) {
-    $res = db_query("SELECT note FROM users_note WHERE userid=$id LIMIT 1");
+    $res = db_query_safe("SELECT note FROM users_note WHERE userid = ? LIMIT 1", [$id]);
     $row = mysql_fetch_assoc($res);
     $cc = "";
     if (preg_match("/\[group: (\w+)\]/", $row["note"], $matches)) {
@@ -316,7 +331,7 @@ function find_group_address_from_notes_for($id) {
 define("MT_USER_APPROVE_MAIL", "group@php.net");
 define("MT_USER_REMOVE_MAIL", "group@php.net");
 function user_approve($id) {
-    $res = db_query("UPDATE users SET cvsaccess=1, enable=1 WHERE userid=$id");
+    $res = db_query_safe("UPDATE users SET cvsaccess=1, enable=1 WHERE userid=?", [$id]);
     if ($res && mysql_affected_rows()) {
       $cc = find_group_address_from_notes_for($id);
       $mailtext = $cc ? $cc : EMAIL_DEFAULT_CC;
@@ -343,7 +358,7 @@ function user_approve($id) {
 
 function user_remove($id) {
     $userinfo = fetch_user($id);
-    $res = db_query("DELETE FROM users WHERE userid=$id");
+    $res = db_query_safe("DELETE FROM users WHERE userid=?", [$id]);
     if ($res && mysql_affected_rows()) {
       $cc = find_group_address_from_notes_for($id);
 
@@ -360,8 +375,8 @@ function user_remove($id) {
 
       /* Notify public records */
       mail($to, $subject, $message,"From: PHP Group <group@php.net>\nIn-Reply-To: <cvs-account-$id@php.net>", "-fnoreply@php.net");
-      db_query("DELETE FROM users_note WHERE userid=$id");
-      db_query("DELETE FROM users_profile WHERE userid=$id");
+      db_query_safe("DELETE FROM users_note WHERE userid=?", [$id]);
+      db_query_safe("DELETE FROM users_profile WHERE userid=?", [$id]);
       warn("record $id ($userinfo[username]) removed");
       return true;
     }
@@ -446,32 +461,23 @@ function is_mirror_site_admin($user) {
 function can_modify($user,$userid) {
   if (is_admin($user)) return true;
 
-  $userid = (int)$userid;
-
-  $quser = addslashes($user);
-  $query = "SELECT userid FROM users"
-         . " WHERE userid=$userid"
-         . "   AND (email='$quser' OR username='$quser')";
-
-  $res = db_query($query);
+  $query = "SELECT userid FROM users WHERE userid = ? AND (email = ? OR username = ?)";
+  $res = db_query_safe($query, [$userid, $user, $user]);
   return $res ? mysql_num_rows($res) : false;
 }
 
 function fetch_user($user) {
-  $query = "SELECT * FROM users LEFT JOIN users_note USING (userid)";
   if ((int)$user) {
-    $query .= " WHERE users.userid=$user";
-  }
-  else {
-    $quser = addslashes((string)$user);
-    $query .= " WHERE username='$quser' OR email='$quser'";
-  }
-
-  if ($res = db_query($query)) {
-    return mysql_fetch_array($res);
+    $res = db_query_safe(
+      "SELECT * FROM users LEFT JOIN users_note USING (userid) WHERE users.userid = ?",
+      [$user]);
+  } else {
+    $res = db_query_safe(
+      "SELECT * FROM users LEFT JOIN users_note USING (userid) WHERE username = ? OR email = ?",
+      [$user, $user]);
   }
 
-  return false;
+  return mysql_fetch_array($res);
 }
 function invalid_input($in) {
   if (!empty($in['email']) && strlen($in['email']) && !is_emailable_address($in['email'])) {
@@ -503,13 +509,8 @@ function validateAction($k) {
 }
 
 function fetch_event($id) {
-  $query = "SELECT * FROM phpcal WHERE id=$id";
-
-  if ($res = db_query($query)) {
-    return mysql_fetch_array($res,MYSQL_ASSOC);
-  }
-
-  return false;
+  $res = db_query_safe("SELECT * FROM phpcal WHERE id = ?", [$id]);
+  return mysql_fetch_array($res,MYSQL_ASSOC);
 }
 
 function display_options($options,$current) {

manage/challenge-response.php

@@ -13,13 +13,14 @@
 
 if (isset($_POST['confirm_them']) && isset($_POST['confirm']) && is_array($_POST['confirm'])) {
 	foreach ($_POST['confirm'] as $address) {
-		$addr = mysql_real_escape_string($address);
-		db_query("insert into accounts.confirmed (email, ts) values ('$addr', NOW())");
+		db_query_safe("insert into accounts.confirmed (email, ts) values (?, NOW())", [$address]);
 	}
 }
 
-$user_db = mysql_real_escape_string($user);
-$res     = db_query("select distinct sender from phpmasterdb.users left join accounts.quarantine on users.email = rcpt where username='$user_db' and not isnull(id)");
+// TODO: Where does $user come from here?
+$res = db_query_safe(
+  "select distinct sender from phpmasterdb.users left join accounts.quarantine on users.email = rcpt " .
+  "where username=? and not isnull(id)", [$user]);
 
 $inmates = [];
 while ($row = mysql_fetch_row($res)) {
@@ -81,7 +82,9 @@ function sort_by_domain($a, $b)
 </form>
 
 <?php
-$res = db_query("select count(id) from phpmasterdb.users left join accounts.quarantine on users.email = rcpt where username='$user_db'");
+$res = db_query_safe(
+  "select count(id) from phpmasterdb.users left join accounts.quarantine on users.email = rcpt " .
+  " where username=?", [$user]);
 
 $n = 0;
 if (mysql_num_rows($res) > 0) {

manage/event.php

@@ -25,16 +25,22 @@
 head("event administration");
 db_connect();
 
-$valid_vars = ['id', 'action','in','begin','max','search','order','full','unapproved'];
-foreach($valid_vars as $k) {
-    $$k = isset($_REQUEST[$k]) ? $_REQUEST[$k] : false;
-}
+$id = isset($_REQUEST['id']) ? $_REQUEST['id'] : false;
+$action = isset($_REQUEST['action']) ? $_REQUEST['action'] : false;
+$in = isset($_REQUEST['in']) ? $_REQUEST['in'] : false;
+$begin = isset($_REQUEST['begin']) ? $_REQUEST['begin'] : false;
+$max = isset($_REQUEST['max']) ? $_REQUEST['max'] : false;
+$search = isset($_REQUEST['search']) ? $_REQUEST['search'] : false;
+$order = isset($_REQUEST['order']) ? $_REQUEST['order'] : false;
+$full = isset($_REQUEST['full']) ? $_REQUEST['full'] : false;
+$unapproved = isset($_REQUEST['unapproved']) ? $_REQUEST['unapproved'] : false;
+
 if($id) $id = (int)$id;
 
 if ($id && $action) {
   switch ($action) {
   case 'approve':
-    if (db_query("UPDATE phpcal SET approved=1,app_by='".real_clean($cuser)."' WHERE id=$id")
+    if (db_query_safe("UPDATE phpcal SET approved=1,app_by=? WHERE id=?", [$cuser, $id])
      && mysql_affected_rows()) {
       $event = fetch_event($id);
       $message = "This event has been approved. It will appear on the PHP website shortly.";
@@ -48,7 +54,7 @@
     break;
   case 'reject':
     $event = fetch_event($id);
-    if (db_query("DELETE FROM phpcal WHERE id=$id")
+    if (db_query_safe("DELETE FROM phpcal WHERE id=?", [$id])
      && mysql_affected_rows()) {
       $message = $event['approved'] ?  "This event has been deleted." : "This event has been rejected.";
       $did = $event['approved'] ? 'Deleted' : 'Rejected';

manage/user-notes.php

@@ -145,8 +145,8 @@
           if (($iprange = wildcard_ip($_GET['votessearch'])) !== false) {
             $search = html_entity_decode($_GET['votessearch'], ENT_QUOTES, 'UTF-8');
             $start = real_clean($iprange[0]); $end = real_clean($iprange[1]);
-            $resultCount = db_query("SELECT count(votes.id) AS total_votes FROM votes JOIN (note) ON (votes.note_id = note.id) WHERE ".
-                                    "(hostip >= $start AND hostip <= $end) OR (ip >= $start AND ip <= $end)");
+            $resultCount = db_query_safe("SELECT count(votes.id) AS total_votes FROM votes JOIN (note) ON (votes.note_id = note.id) WHERE ".
+                                    "(hostip >= ? AND hostip <= ?) OR (ip >= ? AND ip <= ?)", [$start, $end, $start, $end]);
             $resultCount = mysql_fetch_assoc($resultCount);
             $resultCount = $resultCount['total_votes'];
             $isSearch = '&votessearch=' . hsc($search);
@@ -158,7 +158,7 @@
 
           } elseif (filter_var(html_entity_decode($_GET['votessearch'], ENT_QUOTES, 'UTF-8'), FILTER_VALIDATE_IP)) {
             $searchip = (int) ip2long(filter_var(html_entity_decode($_GET['votessearch'], ENT_QUOTES, 'UTF-8'), FILTER_VALIDATE_IP));
-            $resultCount = db_query("SELECT count(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id) WHERE hostip = $searchip OR ip = $searchip");
+            $resultCount = db_query_safe("SELECT count(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id) WHERE hostip = ? OR ip = ?", [$searchip, $searchip]);
             $resultCount = mysql_fetch_assoc($resultCount);
             $resultCount = $resultCount['total_votes'];
             $isSearch = '&votessearch=' . hsc(long2ip($searchip));
@@ -169,7 +169,7 @@
                    "ORDER BY votes.id DESC LIMIT $limitVotes, 25";
           } else {
             $search = (int) html_entity_decode($_GET['votessearch'], ENT_QUOTES, 'UTF-8');
-            $resultCount = db_query("SELECT count(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id) WHERE votes.note_id = $search");
+            $resultCount = db_query_safe("SELECT count(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id) WHERE votes.note_id = ?", [$search]);
             $resultCount = mysql_fetch_assoc($resultCount);
             $resultCount = $resultCount['total_votes'];
             $isSearch = '&votessearch=' . hsc($search);
@@ -181,7 +181,7 @@
           }
         } else {
           $isSearch = null;
-          $resultCount = db_query("SELECT COUNT(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id)");
+          $resultCount = db_query_safe("SELECT COUNT(votes.id) AS total_votes FROM votes JOIN(note) ON (votes.note_id = note.id)");
           $resultCount = mysql_fetch_assoc($resultCount);
           $resultCount = $resultCount['total_votes'];
           $sql = "SELECT votes.id, UNIX_TIMESTAMP(votes.ts) AS ts, votes.vote, votes.note_id, note.sect, votes.hostip, votes.ip ".
@@ -513,7 +513,7 @@
         die ("Note #$id has already been approved");
       }
 
-      if ($row['id'] && db_query("UPDATE note SET status=NULL WHERE id=".real_clean($id))) {
+      if ($row['id'] && db_query_safe("UPDATE note SET status=NULL WHERE id=?", [$id])) {
         note_mail_on_action(
             $cuser,
             $id,
@@ -530,7 +530,7 @@
 case 'delete':
   if ($id) {
     if ($row = note_get_by_id($id)) {
-      if ($row['id'] && db_query("DELETE note,votes FROM note LEFT JOIN (votes) ON (note.id = votes.note_id) WHERE note.id = ".real_clean($id))) {
+      if ($row['id'] && db_query_safe("DELETE note,votes FROM note LEFT JOIN (votes) ON (note.id = votes.note_id) WHERE note.id = ?", [$id])) {
         // ** alerts **
         //$mailto .= get_emails_for_sect($row["sect"]);
         $action_taken = ($action == "reject" ? "rejected" : "deleted");
@@ -647,7 +647,7 @@
         $sql = 'DELETE FROM votes WHERE votes.note_id = ' . real_clean($id) . ' AND votes.vote = 0';
       }
       /* Make sure the note has votes before we attempt to delete them */
-      $result = db_query("SELECT COUNT(id) AS id FROM votes WHERE note_id = " . real_clean($id));
+      $result = db_query_safe("SELECT COUNT(id) AS id FROM votes WHERE note_id = ?", [$id]);
       $rows = mysql_fetch_assoc($result);
       if (!$rows['id']) {
         echo "<p>No votes exist for Note ID ". hsc($id) ."!</p>";

manage/users.php

@@ -61,18 +61,17 @@ function csrf_validate(&$mydata, $name) {
 
 # ?username=whatever will look up 'whatever' by email or username
 if ($username) {
-  $tmp = filter_input(INPUT_GET, "username", FILTER_CALLBACK, ["options" => "mysql_real_escape_string"]) ?: "";
   $query = "SELECT userid FROM users"
-         . " WHERE username='$tmp' OR email='$tmp'";
-  $res = db_query($query);
+         . " WHERE username=? OR email=?";
+  $res = db_query_safe($query, [$username, $username]);
 
   if (!($id = @mysql_result($res, 0))) {
     warn("wasn't able to find user matching '$username'");
   }
 }
 if ($id) {
-  $query = "SELECT * FROM users WHERE users.userid=$id";
-  $res = db_query($query);
+  $query = "SELECT * FROM users WHERE users.userid=?";
+  $res = db_query_safe($query, [$id]);
   $userdata = mysql_fetch_array($res);
   if (!$userdata) {
     warn("Can't find user#$id");
@@ -249,7 +248,7 @@ function csrf_validate(&$mydata, $name) {
 </tr>
 <?php
   if ($id) {
-    $res = db_query("SELECT markdown FROM users_profile WHERE userid=$id");
+    $res = db_query_safe("SELECT markdown FROM users_profile WHERE userid=?", [$id]);
     $userdata['profile_markdown'] = '';
     if ($profile_row = mysql_fetch_assoc($res)) {
         $userdata['profile_markdown'] = $profile_row['markdown'];
@@ -305,7 +304,7 @@ function csrf_validate(&$mydata, $name) {
 ?>
 <h2 id="notes">Notes:</h2>
 <?php
-  $res = db_query("SELECT note, UNIX_TIMESTAMP(entered) AS ts FROM users_note WHERE userid=$id");
+  $res = db_query_safe("SELECT note, UNIX_TIMESTAMP(entered) AS ts FROM users_note WHERE userid=?", [$id]);
   while ($res && $userdata = mysql_fetch_assoc($res)) {
     echo "<div class='note'>", date("r",$userdata['ts']), "<br />".$userdata['note']."</div>";
   }
@@ -350,7 +349,7 @@ function csrf_validate(&$mydata, $name) {
 $res = db_query($query);
 #echo $query;
 
-$res2 = db_query("SELECT FOUND_ROWS()");
+$res2 = db_query_safe("SELECT FOUND_ROWS()");
 $total = (int)mysql_result($res2,0);