From 4e558e5e7c391c8fc81b7bba1b3ed2019ff8b061 Mon Sep 17 00:00:00 2001 From: Emmanuel Monbroussou Date: Wed, 18 May 2016 12:51:03 +0200 Subject: [PATCH] [wip] Passing mysql request params into array arguments for the query method (part 4) --- bureau/class/db_mysql.php | 9 +++++ bureau/class/m_admin.php | 43 +++++++++++++---------- bureau/class/m_authip.php | 4 +-- bureau/class/m_bro.php | 1 - bureau/class/m_dom.php | 73 ++++++++++++++++++++++----------------- bureau/class/m_mail.php | 18 +++++----- bureau/class/m_mysql.php | 25 +++++--------- 7 files changed, 96 insertions(+), 77 deletions(-) diff --git a/bureau/class/db_mysql.php b/bureau/class/db_mysql.php index c23d1d72..5f307974 100644 --- a/bureau/class/db_mysql.php +++ b/bureau/class/db_mysql.php @@ -271,6 +271,15 @@ class DB_Sql { return $this->pdo_instance->lastInsertId(); } + /** + * Escape a string to use it into a SQL PDO query + * @param string string to escape + * @return string escaped string + */ + function quote($string) { + return $this->pdo_instance->quote($string); + } + /* public: sequence numbers */ function nextid($seq_name) { if (!$this->is_connected()) diff --git a/bureau/class/m_admin.php b/bureau/class/m_admin.php index f1b7ad39..1e817c72 100644 --- a/bureau/class/m_admin.php +++ b/bureau/class/m_admin.php @@ -752,32 +752,29 @@ EOF; function update_mem($uid, $mail, $nom, $prenom, $pass, $enabled, $canpass, $type = 'default', $duration = 0, $notes = "", $reset_quotas = false) { global $err, $db, $quota; - $notes = addslashes($notes); - $err->log("admin", "update_mem", $uid); + if (!$this->enabled) { $err->raise("admin", _("-- Only administrators can access this page! --")); return false; } $db = new DB_System(); - // @TODO:EM: this has to be escaped + if ($pass) { $pass = _md5cr($pass); - $ssq = " ,pass='$pass' "; + $second_query = "UPDATE membres SET mail= ?, canpass= ?, enabled= ?, `type`= ?, notes= ? , pass = ? WHERE uid= ?;"; + $second_query_args = array($mail, $canpass, $enabled, $type, $notes, $pass, $uid); } else { - $ssq = ""; + $second_query = "UPDATE membres SET mail= ?, canpass= ?, enabled= ?, `type`= ?, notes= ? WHERE uid= ?;"; + $second_query_args = array($mail, $canpass, $enabled, $type, $notes, $uid); } $old_mem = $this->get($uid); if( - ($db->query( - "UPDATE local SET nom= ?, prenom= ? WHERE uid=?;", - array($nom, $prenom, $uid) - )) && - ($db->query( - "UPDATE membres SET mail= ?, canpass= ?, enabled= ?, `type`= ?, notes= ? $ssq WHERE uid= ?;", - array($mail, $canpass, $enabled, $type, $notes, $uid)))) { + ($db->query("UPDATE local SET nom= ?, prenom= ? WHERE uid=?;", array($nom, $prenom, $uid))) && + ($db->query($second_query, $second_query_args)) + ){ if ($reset_quotas == "on" || $type != $old_mem['type']) { $quota->addquotas(); $quota->synchronise_user_profile(); @@ -1105,9 +1102,14 @@ EOF; } } - // @TODO:EM: this has to be escaped - $filter=($hosting_tld=variable_get("hosting_tld")) ? " WHERE domaine not like '%.$hosting_tld'" : ""; - $db->query("SELECT m.uid,m.login,d.domaine,d.gesdns,d.gesmx,d.noerase FROM domaines d LEFT JOIN membres m ON m.uid=d.compte $filter ORDER BY domaine;"); + $query = "SELECT m.uid,m.login,d.domaine,d.gesdns,d.gesmx,d.noerase FROM domaines d LEFT JOIN membres m ON m.uid=d.compte "; + $query_args = array(); + if($hosting_tld = variable_get("hosting_tld")){ + $query .= " WHERE domaine not like ?"; + array_push($query_args, "%.".$hosting_tld); + } + $query .= " ORDER BY domaine;"; + $db->query($query, $query_args); $c = array(); while ($db->next_record()) { $tmp = $db->Record; @@ -1134,9 +1136,14 @@ EOF; global $db, $L_NS1, $L_NS2, $L_MX, $L_PUBLIC_IP; $checked = array(); - // @TODO:EM: this has to be escaped - $filter=($hosting_tld=variable_get("hosting_tld")) ? " WHERE domaine not like '%.$hosting_tld'" : ""; - $db->query("SELECT * FROM domaines $filter ORDER BY domaine"); + $query = "SELECT * FROM domaines "; + $query_args = array(); + if($hosting_tld = variable_get("hosting_tld")){ + $query .= " WHERE domaine not like ?"; + array_push($query_args, "%.".$hosting_tld); + } + $query .= " ORDER BY domaine"; + $db->query($query, $query_args); $dl = array(); while ($db->next_record()) { $dl[$db->Record["domaine"]] = $db->Record; diff --git a/bureau/class/m_authip.php b/bureau/class/m_authip.php index 63749000..0f8c74bf 100644 --- a/bureau/class/m_authip.php +++ b/bureau/class/m_authip.php @@ -249,7 +249,7 @@ class m_authip { foreach ($list_affected as $k => $v) { $this->call_hooks("authip_on_delete", $k); } - if (!$db->query("update authorised_ip set ip= ?, subnet= ?, infos= ? where id= ? and uid=? ;", array($id, $subnetn $infos, $id, $cuid)) { + if (!$db->query("update authorised_ip set ip= ?, subnet= ?, infos= ? where id= ? and uid=? ;", array($id, $subnetn, $infos, $id, $cuid))) { echo "query failed: " . $db->Error; return false; } @@ -329,7 +329,7 @@ class m_authip { echo "query failed: " . $db->Error; return false; } - $this->call_hooks("authip_on_create", PDO::lastInsertId()); // @TODO:EM: To test + $this->call_hooks("authip_on_create", $db->lastid()); } return true; } diff --git a/bureau/class/m_bro.php b/bureau/class/m_bro.php index e07425b1..556bb957 100644 --- a/bureau/class/m_bro.php +++ b/bureau/class/m_bro.php @@ -839,7 +839,6 @@ class m_bro { $beg = $dir; $tofind = true; while ($tofind) { - // @TODO:EM: be careful with this one! $db->query("SELECT sub,domaine FROM sub_domaines WHERE compte= ? AND type=0 AND (valeur= ? or valeur= ?);", array($cuid, "/".$beg."/", "/".$beg)); $db->next_record(); if ($db->num_rows()) { diff --git a/bureau/class/m_dom.php b/bureau/class/m_dom.php index e196955b..2d6b9540 100644 --- a/bureau/class/m_dom.php +++ b/bureau/class/m_dom.php @@ -1682,17 +1682,21 @@ class m_dom { /* ----------------------------------------------------------------- */ /** Returns the complete hosted domain list : - * @TODO:EM: this has to be escaped */ function get_domain_list($uid = -1) { global $db; $uid = intval($uid); $res = array(); $sql = ""; + + $query = "SELECT domaine FROM domaines WHERE gesdns=1 "; + $query_args = array(); if ($uid != -1) { - $sql .= " AND compte='$uid' "; + $query .= " AND compte= ? "; + array_push($query_args, $uid); } - $db->query("SELECT domaine FROM domaines WHERE gesdns=1 $sql ORDER BY domaine"); + $query .= " ORDER BY domaine;"; + $db->query($query, $query_args); while ($db->next_record()) { $res[] = $db->f("domaine"); } @@ -1964,45 +1968,50 @@ class m_dom { * of a vhost. * If no parameters, return the parameters for ALL the vhost. * Optionnal parameters: id of the sub_domaines - * - * @TODO:EM: This has to be escaped * */ function generation_parameters($id = null, $only_apache = true) { global $db, $err; $err->log("dom", "generation_parameters"); $params = ""; + /** 2016_05_18 : this comments was here before escaping the request... is there still something to do here ? + * // BUG BUG BUG FIXME + * // Suppression de comptes -> membres existe pas -> domaines a supprimer ne sont pas lister + */ + $query = " + select + sd.id as sub_id, + lower(sd.type) as type, + m.login, + m.uid as uid, + if(length(sd.sub)>0,concat_ws('.',sd.sub,sd.domaine),sd.domaine) as fqdn, + concat_ws('@',m.login,v.value) as mail, + sd.valeur + from + sub_domaines sd left join membres m on sd.compte=m.uid, + variable v, + domaines_type dt + where + v.name='mailname_bounce' + and lower(dt.name) = lower(sd.type)"; + $query_args = array(); + if (!is_null($id) && intval($id) == $id) { - $id = intval($id); - $params = " AND sd.id = $id "; + $query .= " AND sd.id = ? "; + array_push($query_args, intval($id)); } if ($only_apache) { - $params.=" and dt.only_dns is false "; + $query .=" and dt.only_dns is false "; } + + $query .= " + order by + m.login, + sd.domaine, + sd.sub;"; + -// BUG BUG BUG FIXME -// Suppression de comptes -> membres existe pas -> domaines a supprimer ne sont pas lister - $db->query(" -select - sd.id as sub_id, - lower(sd.type) as type, - m.login, - m.uid as uid, - if(length(sd.sub)>0,concat_ws('.',sd.sub,sd.domaine),sd.domaine) as fqdn, - concat_ws('@',m.login,v.value) as mail, - sd.valeur -from - sub_domaines sd left join membres m on sd.compte=m.uid, - variable v, - domaines_type dt -where - v.name='mailname_bounce' - and lower(dt.name) = lower(sd.type) - $params -order by - m.login, - sd.domaine, - sd.sub -;"); + $db->query($query, $query_args); + $r = array(); while ($db->next_record()) { $r[$db->Record['sub_id']] = $db->Record; diff --git a/bureau/class/m_mail.php b/bureau/class/m_mail.php index 15be9e4a..2fbf99ea 100644 --- a/bureau/class/m_mail.php +++ b/bureau/class/m_mail.php @@ -290,33 +290,35 @@ ORDER BY * @param $offset integer skip THAT much emails in the result. * @param $count integer return no more than THAT much emails. -1 for ALL. Offset is ignored then. * @result an array of each mail hosted under the domain. - * @TODO:EM: It has to be escaped */ function enum_domain_mails($dom_id = null, $search = "", $offset = 0, $count = 30, $show_systemmails = false) { global $db, $err, $hooks; $err->log("mail", "enum_domains_mail"); - $search = trim($search); + $query_args = array($dom_id); + $search = trim($search); + $where = " a.domain_id = ? "; - $where = "a.domain_id=$dom_id"; if ($search) { - $where.=" AND (a.address LIKE '%" . addslashes($search) . "%' OR r.recipients LIKE '%" . addslashes($search) . "%')"; + $where .= " AND (a.address LIKE ? OR r.recipients LIKE ? )"; + array_push($query_args, "%" . $search . "%", "%" . $search . "%"); } if (!$show_systemmails) { - $where.=" AND type='' "; + $where .= " AND type='' "; } - $db->query("SELECT count(a.id) AS total FROM address a LEFT JOIN recipient r ON r.address_id=a.id WHERE $where;"); + $db->query("SELECT count(a.id) AS total FROM address a LEFT JOIN recipient r ON r.address_id=a.id WHERE " . $where . ";", $query_args); $db->next_record(); $this->total = $db->f("total"); if ($count != -1) { - $limit = "LIMIT $offset,$count"; + $limit = " LIMIT ?, ? "; + array_push($query_args, $offset, $count); } else { $limit = ""; } $db->query("SELECT a.id, a.address, a.password, a.`enabled`, a.mail_action, d.domaine AS domain, m.quota, m.quota*1024*1024 AS quotabytes, m.bytes AS used, NOT ISNULL(m.id) AS islocal, a.type, r.recipients, m.lastlogin, a.domain_id FROM (address a LEFT JOIN mailbox m ON m.address_id=a.id) LEFT JOIN recipient r ON r.address_id=a.id, domaines d - WHERE $where AND d.id=a.domain_id $limit ;"); + WHERE " . $where . " AND d.id=a.domain_id " . $limit . " ;", $query_args); if (!$db->next_record()) { $err->raise("mail", _("No email found for this query")); return array(); diff --git a/bureau/class/m_mysql.php b/bureau/class/m_mysql.php index dbb3785f..2fb52ad5 100644 --- a/bureau/class/m_mysql.php +++ b/bureau/class/m_mysql.php @@ -479,15 +479,14 @@ class m_mysql { return false; } - // @TODO:EM: does this part have to be escaped? # Protect database name if not wildcard if ($base != '*') { - $base = "`" . $base . "`"; + $base = $db->quote($base); } - $grant = "grant " . $rights . " on " . $base . "." . $table . " to '" . $user . "'@'" . $this->dbus->Client . "'"; + $grant = "grant " . $db->quote($rights) . " on " . $base . "." . $db->quote($table) . " to " . $db->quote($user) . "@" . $db->quote($this->dbus->Client); if ($pass) { - $grant .= " identified by '" . $pass . "';"; + $grant .= " identified by " . $db->quote($pass) . ";"; } else { $grant .= ";"; } @@ -556,8 +555,7 @@ class m_mysql { * @access private */ function get_db_size($dbname) { - // @TODO:EM: does this part have to be escaped? - $this->dbus->query("SHOW TABLE STATUS FROM `$dbname`;"); + $this->dbus->query("SHOW TABLE STATUS FROM ". $db->quote($dbname) .";"); $size = 0; while ($this->dbus->next_record()) { $size += $this->dbus->f('Data_length') + $this->dbus->f('Index_length'); @@ -753,7 +751,6 @@ class m_mysql { * @param $password The password for this username * @param $passconf The password confirmation * @return boolean if the password has been changed in MySQL or FALSE if an error occurred - * @TODO:EM: is this correctly escaped ? * */ function change_user_password($usern, $password, $passconf) { global $db, $err, $cuid, $admin; @@ -771,7 +768,7 @@ class m_mysql { return false; // The error has been raised by checkPolicy() } } - $this->dbus->query("SET PASSWORD FOR '" . addslashes($usern) . "'@'" . $this->dbus->Client . "' = PASSWORD(?);", array($pass)); + $this->dbus->query("SET PASSWORD FOR " . $db->quote($usern) . "@" . $db->quote($this->dbus->Client) . "' = PASSWORD(?);", array($pass)); $db->query("UPDATE dbusers set password= ? where name= ? and uid= ? ;", array($pass, $usern, $cuid)); return true; } @@ -805,8 +802,7 @@ class m_mysql { $login = $db->f("name"); // Ok, database exists and dbname is compliant. Let's proceed - // @TODO:EM: is this correctly escaped ? - $this->dbus->query("REVOKE ALL PRIVILEGES ON *.* FROM '" . $user . "'@'" . $this->dbus->Client . "';"); + $this->dbus->query("REVOKE ALL PRIVILEGES ON *.* FROM " . $db->quote($user) . "@" . $db->quote($this->dbus->Client) . ";"); $this->dbus->query("DELETE FROM mysql.db WHERE User= ? AND Host= ? ;", array($user, $this->dbus->Client)); $this->dbus->query("DELETE FROM mysql.user WHERE User= ? AND Host= ? ;", array($user, $this->dbus->Client)); $this->dbus->query("FLUSH PRIVILEGES"); @@ -873,7 +869,6 @@ class m_mysql { global $err; $err->log("mysql", "set_user_rights"); - $dbname = str_replace('_', '\_', $dbname); // On genere les droits en fonction du tableau de droits $strrights = ""; for ($i = 0; $i < count($rights); $i++) { @@ -936,18 +931,16 @@ class m_mysql { } // We reset all user rights on this DB : - $this->dbus->query("SELECT * FROM mysql.db WHERE User = ? AND Db = ?;", array($usern, $dbname)); + $this->dbus->query("SELECT * FROM mysql.db WHERE User = ? AND Db = ?;", array($user, $dbn)); // @TODO:EM: This has to be verified, and maybe we should use another way to escape those requests - $usern = addslashes($user); - $dbname = addslashes($dbn); if ($this->dbus->num_rows()) { - $this->dbus->query("REVOKE ALL PRIVILEGES ON `$dbname`.* FROM '$usern'@'" . $this->dbus->Client . "';"); + $this->dbus->query("REVOKE ALL PRIVILEGES ON ".$db->quote($dbn).".* FROM ".$db->quote($user)."@" . $db->quote($this->dbus->Client) . ";"); } if ($strrights) { $strrights = substr($strrights, 0, strlen($strrights) - 1); - $this->grant($dbname, $usern, $strrights); + $this->grant($dbn, $user, $strrights); } $this->dbus->query("FLUSH PRIVILEGES"); return TRUE;