diff --git a/htdocs/comm/action/index.php b/htdocs/comm/action/index.php index 6c92f07085b..b8d443d8096 100644 --- a/htdocs/comm/action/index.php +++ b/htdocs/comm/action/index.php @@ -115,8 +115,8 @@ if ($dateselect > 0) { } // Set actioncode (this code must be same for setting actioncode into peruser, listacton and index) -if (GETPOST('search_actioncode', 'array')) { - $actioncode = GETPOST('search_actioncode', 'array', 3); +if (GETPOST('search_actioncode', 'array:aZ09')) { + $actioncode = GETPOST('search_actioncode', 'array:aZ09', 3); if (!count($actioncode)) { $actioncode = '0'; } @@ -669,18 +669,18 @@ if (!empty($actioncode)) { $sql .= " AND ca.type = 'systemauto'"; } else { if (is_array($actioncode)) { - $sql .= " AND ca.code IN ('".implode("','", $actioncode)."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", $actioncode)."'", 1).")"; } else { - $sql .= " AND ca.code IN ('".implode("','", explode(',', $actioncode))."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", explode(',', $actioncode))."'", 1).")"; } } } } if ($resourceid > 0) { - $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".$db->escape($resourceid); + $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".((int) $resourceid); } if ($pid) { - $sql .= " AND a.fk_project=".$db->escape($pid); + $sql .= " AND a.fk_project=".((int) $pid); } if (!$user->rights->societe->client->voir && !$socid) { $sql .= " AND (a.fk_soc IS NULL OR sc.fk_user = ".$user->id.")"; diff --git a/htdocs/comm/action/list.php b/htdocs/comm/action/list.php index 64ed79d87ca..888a13fb4b8 100644 --- a/htdocs/comm/action/list.php +++ b/htdocs/comm/action/list.php @@ -429,31 +429,31 @@ if (!empty($actioncode)) { $sql .= " AND c.type = 'systemauto'"; } else { if (is_array($actioncode)) { - $sql .= " AND c.code IN ('".implode("','", $actioncode)."')"; + $sql .= " AND c.code IN (".$db->sanitize("'".implode("','", $actioncode)."'", 1).")"; } else { - $sql .= " AND c.code IN ('".implode("','", explode(',', $actioncode))."')"; + $sql .= " AND c.code IN (".$db->sanitize("'".implode("','", explode(',', $actioncode))."'", 1).")"; } } } } if ($resourceid > 0) { - $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".$db->escape($resourceid); + $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".((int) $resourceid); } if ($pid) { - $sql .= " AND a.fk_project=".$db->escape($pid); + $sql .= " AND a.fk_project=".((int) $pid); } if (!$user->rights->societe->client->voir && !$socid) { $sql .= " AND (a.fk_soc IS NULL OR sc.fk_user = ".$user->id.")"; } if ($socid > 0) { - $sql .= " AND s.rowid = ".$socid; + $sql .= " AND s.rowid = ".((int) $socid); } // We must filter on assignement table if ($filtert > 0 || $usergroup > 0) { $sql .= " AND ar.fk_actioncomm = a.id AND ar.element_type='user'"; } if ($type) { - $sql .= " AND c.id = ".(int) $type; + $sql .= " AND c.id = ".((int) $type); } if ($search_status == '0') { $sql .= " AND a.percent = 0"; @@ -486,10 +486,10 @@ if ($search_note) { if ($filtert > 0 || $usergroup > 0) { $sql .= " AND ("; if ($filtert > 0) { - $sql .= "(ar.fk_element = ".$filtert." OR (ar.fk_element IS NULL AND a.fk_user_action=".$filtert."))"; // The OR is for backward compatibility + $sql .= "(ar.fk_element = ".((int) $filtert)." OR (ar.fk_element IS NULL AND a.fk_user_action = ".((int) $filtert)."))"; // The OR is for backward compatibility } if ($usergroup > 0) { - $sql .= ($filtert > 0 ? " OR " : "")." ugu.fk_usergroup = ".$usergroup; + $sql .= ($filtert > 0 ? " OR " : "")." ugu.fk_usergroup = ".((int) $usergroup); } $sql .= ")"; } diff --git a/htdocs/comm/action/pertype.php b/htdocs/comm/action/pertype.php index dc70fb30743..3b55399a58a 100644 --- a/htdocs/comm/action/pertype.php +++ b/htdocs/comm/action/pertype.php @@ -540,24 +540,24 @@ if (!empty($actioncode)) { $sql .= " AND ca.type = 'systemauto'"; } else { if (is_array($actioncode)) { - $sql .= " AND ca.code IN ('".implode("','", $actioncode)."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", $actioncode)."'", 1).")"; } else { - $sql .= " AND ca.code IN ('".implode("','", explode(',', $actioncode))."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", explode(',', $actioncode))."'", 1).")"; } } } } if ($resourceid > 0) { - $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".$db->escape($resourceid); + $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".((int) $resourceid); } if ($pid) { - $sql .= " AND a.fk_project=".$db->escape($pid); + $sql .= " AND a.fk_project=".((int) $pid); } if (!$user->rights->societe->client->voir && !$socid) { $sql .= " AND (a.fk_soc IS NULL OR sc.fk_user = ".$user->id.")"; } if ($socid > 0) { - $sql .= ' AND a.fk_soc = '.$socid; + $sql .= ' AND a.fk_soc = '.((int) $socid); } // We must filter on assignement table if ($filtert > 0 || $usergroup > 0) { diff --git a/htdocs/comm/action/peruser.php b/htdocs/comm/action/peruser.php index 388363f63fc..8c440395083 100644 --- a/htdocs/comm/action/peruser.php +++ b/htdocs/comm/action/peruser.php @@ -105,8 +105,8 @@ $type = GETPOST("search_type", 'alpha') ?GETPOST("search_type", 'alpha') : GETPO $maxprint = ((GETPOST("maxprint", 'int') != '') ?GETPOST("maxprint", 'int') : $conf->global->AGENDA_MAX_EVENTS_DAY_VIEW); $optioncss = GETPOST('optioncss', 'aZ'); // Option for the css output (always '' except when 'print') // Set actioncode (this code must be same for setting actioncode into peruser, listacton and index) -if (GETPOST('search_actioncode', 'array')) { - $actioncode = GETPOST('search_actioncode', 'array', 3); +if (GETPOST('search_actioncode', 'array:aZ09')) { + $actioncode = GETPOST('search_actioncode', 'array:aZ09', 3); if (!count($actioncode)) { $actioncode = '0'; } @@ -562,24 +562,24 @@ if (!empty($actioncode)) { $sql .= " AND ca.type = 'systemauto'"; } else { if (is_array($actioncode)) { - $sql .= " AND ca.code IN ('".implode("','", $actioncode)."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", $actioncode)."'", 1).")"; } else { - $sql .= " AND ca.code IN ('".implode("','", explode(',', $actioncode))."')"; + $sql .= " AND ca.code IN (".$db->sanitize("'".implode("','", explode(',', $actioncode))."'", 1).")"; } } } } if ($resourceid > 0) { - $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".$db->escape($resourceid); + $sql .= " AND r.element_type = 'action' AND r.element_id = a.id AND r.resource_id = ".((int) $resourceid); } if ($pid) { - $sql .= " AND a.fk_project=".$db->escape($pid); + $sql .= " AND a.fk_project = ".((int) $pid); } if (!$user->rights->societe->client->voir && !$socid) { $sql .= " AND (a.fk_soc IS NULL OR sc.fk_user = ".$user->id.")"; } if ($socid > 0) { - $sql .= ' AND a.fk_soc = '.$socid; + $sql .= ' AND a.fk_soc = '.((int) $socid); } // We must filter on assignement table if ($filtert > 0 || $usergroup > 0) { diff --git a/htdocs/comm/mailing/class/advtargetemailing.class.php b/htdocs/comm/mailing/class/advtargetemailing.class.php index 8de98f67ebe..063fb6c7f00 100644 --- a/htdocs/comm/mailing/class/advtargetemailing.class.php +++ b/htdocs/comm/mailing/class/advtargetemailing.class.php @@ -568,7 +568,7 @@ class AdvanceTargetingMailing extends CommonObject $sqlwhere[] = " (t.fk_stcomm IN (".$this->db->sanitize(implode(',', $arrayquery['cust_comm_status']))."))"; } if (!empty($arrayquery['cust_prospect_status']) && count($arrayquery['cust_prospect_status']) > 0) { - $sqlwhere[] = " (t.fk_prospectlevel IN ('".$this->db->sanitize(implode("','", $arrayquery['cust_prospect_status']))."'))"; + $sqlwhere[] = " (t.fk_prospectlevel IN (".$this->db->sanitize("'".implode("','", $arrayquery['cust_prospect_status'])."'", 1)."))"; } if (!empty($arrayquery['cust_typeent']) && count($arrayquery['cust_typeent']) > 0) { $sqlwhere[] = " (t.fk_typent IN (".$this->db->sanitize(implode(',', $arrayquery['cust_typeent']))."))"; @@ -586,7 +586,7 @@ class AdvanceTargetingMailing extends CommonObject $sqlwhere[] = " (custcateg.fk_categorie IN (".$this->db->sanitize(implode(',', $arrayquery['cust_categ']))."))"; } if (!empty($arrayquery['cust_language']) && count($arrayquery['cust_language']) > 0) { - $sqlwhere[] = " (t.default_lang IN ('".$this->db->sanitize(implode("','", $arrayquery['cust_language']))."'))"; + $sqlwhere[] = " (t.default_lang IN (".$this->db->sanitize("'".implode("','", $arrayquery['cust_language'])."'", 1)."))"; } //Standard Extrafield feature @@ -618,7 +618,7 @@ class AdvanceTargetingMailing extends CommonObject } } else { if (is_array($arrayquery['options_'.$key])) { - $sqlwhere[] = " (te.".$key." IN ('".implode("','", $arrayquery['options_'.$key])."'))"; + $sqlwhere[] = " (te.".$key." IN (".$this->db->sanitize("'".implode("','", $arrayquery['options_'.$key])."'", 1)."))"; } elseif (!empty($arrayquery['options_'.$key])) { $sqlwhere[] = " (te.".$key." LIKE '".$this->db->escape($arrayquery['options_'.$key])."')"; } @@ -703,7 +703,7 @@ class AdvanceTargetingMailing extends CommonObject $sqlwhere[] = " (t.statut IN (".$this->db->sanitize($this->db->escape(implode(',', $arrayquery['contact_status'])))."))"; } if (!empty($arrayquery['contact_civility']) && count($arrayquery['contact_civility']) > 0) { - $sqlwhere[] = " (t.civility IN ('".$this->db->sanitize($this->db->escape(implode("','", $arrayquery['contact_civility'])))."'))"; + $sqlwhere[] = " (t.civility IN (".$this->db->sanitize("'".implode("','", $arrayquery['contact_civility'])."'", 1)."))"; } if ($arrayquery['contact_no_email'] != '') { $tmpwhere = ''; @@ -762,7 +762,7 @@ class AdvanceTargetingMailing extends CommonObject } } else { if (is_array($arrayquery['options_'.$key.'_cnct'])) { - $sqlwhere[] = " (te.".$key." IN ('".implode("','", $arrayquery['options_'.$key.'_cnct'])."'))"; + $sqlwhere[] = " (te.".$key." IN (".$this->db->sanitize("'".implode("','", $arrayquery['options_'.$key.'_cnct'])."'", 1)."))"; } elseif (!empty($arrayquery['options_'.$key.'_cnct'])) { $sqlwhere[] = " (te.".$key." LIKE '".$this->db->escape($arrayquery['options_'.$key.'_cnct'])."')"; } @@ -860,7 +860,7 @@ class AdvanceTargetingMailing extends CommonObject } } else { if (is_array($arrayquery['options_'.$key])) { - $sqlwhere[] = " (tse.".$key." IN ('".implode("','", $arrayquery['options_'.$key])."'))"; + $sqlwhere[] = " (tse.".$key." IN (".$this->db->sanitize("'".implode("','", $arrayquery['options_'.$key])."'", 1)."))"; } elseif (!empty($arrayquery['options_'.$key])) { $sqlwhere[] = " (tse.".$key." LIKE '".$this->db->escape($arrayquery['options_'.$key])."')"; } diff --git a/htdocs/core/modules/mailings/thirdparties_services_expired.modules.php b/htdocs/core/modules/mailings/thirdparties_services_expired.modules.php index 2718a33247b..7b5d4f38737 100644 --- a/htdocs/core/modules/mailings/thirdparties_services_expired.modules.php +++ b/htdocs/core/modules/mailings/thirdparties_services_expired.modules.php @@ -197,7 +197,7 @@ class mailing_thirdparties_services_expired extends MailingTargets $sql .= " WHERE s.entity IN (".getEntity('societe').")"; $sql .= " AND s.rowid = c.fk_soc AND cd.fk_contrat = c.rowid AND s.email != ''"; $sql .= " AND cd.statut= 4 AND cd.fk_product=p.rowid"; - $sql .= " AND p.ref IN ('".join("','", $this->arrayofproducts)."')"; + $sql .= " AND p.ref IN (".$this->db->sanitize("'".join("','", $this->arrayofproducts)."'", 1).")"; $sql .= " AND cd.date_fin_validite < '".$this->db->idate($now)."'"; $a = parent::getNbOfRecipients($sql); diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 42470a1ddb6..b3250b9e265 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -311,7 +311,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase $this->assertTrue($ok, 'Found non escaped string in building of a sql request '.$file['relativename'].': '.$val[0].' - Bad.'); //exit; - // Check string IN (".xxx with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. + // Check string 'IN (".xxx' or 'IN (\'.xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. preg_match_all('/ IN \([\'"]\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) { @@ -324,6 +324,19 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); //exit; + // Check string 'IN (\'".xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. + preg_match_all('/ IN \(\'"\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { + if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) { + $ok=false; + break; + } + //if ($reg[0] != 'db') $ok=false; + } + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); + //exit; + // Test that output of $_SERVER\[\'QUERY_STRING\'\] is escaped. $ok=true; $matches=array();