diff --git a/htdocs/accountancy/class/accountancysystem.class.php b/htdocs/accountancy/class/accountancysystem.class.php index 87d1ccd663c1660272855e9454f7639a0129b534..68beff8d043b042f92a2117235c3092256b22562 100644 --- a/htdocs/accountancy/class/accountancysystem.class.php +++ b/htdocs/accountancy/class/accountancysystem.class.php @@ -37,7 +37,7 @@ class AccountancySystem var $label; var $account_number; var $account_parent; - + /** * Constructor * @@ -46,8 +46,8 @@ class AccountancySystem function __construct($db) { $this->db = $db; } - - + + /** * Load record in memory * @@ -55,11 +55,11 @@ class AccountancySystem * @param string $ref ref * @return int <0 if KO, Id of record if OK and found */ - function fetch($rowid = 0, $ref = '') + function fetch($rowid = 0, $ref = '') { global $conf; - - if ($rowid > 0 || $ref) + + if ($rowid > 0 || $ref) { $sql = "SELECT a.pcg_version, a.label, a.active"; $sql .= " FROM " . MAIN_DB_PREFIX . "accounting_system as a"; @@ -69,12 +69,12 @@ class AccountancySystem } elseif ($ref) { $sql .= " a.pcg_version = '" . $ref . "'"; } - + dol_syslog(get_class($this) . "::fetch sql=" . $sql, LOG_DEBUG); $result = $this->db->query($sql); if ($result) { $obj = $this->db->fetch_object($result); - + if ($obj) { $this->id = $obj->rowid; $this->rowid = $obj->rowid; @@ -82,7 +82,7 @@ class AccountancySystem $this->ref = $obj->pcg_version; $this->label = $obj->label; $this->active = $obj->active; - + return $this->id; } else { return 0; @@ -94,8 +94,8 @@ class AccountancySystem } return - 1; } - - + + /** * Insert accountancy system name into database * @@ -104,16 +104,16 @@ class AccountancySystem */ function create($user) { $now = dol_now(); - + $sql = "INSERT INTO " . MAIN_DB_PREFIX . "accounting_system"; $sql .= " (date_creation, fk_user_author, numero, label)"; - $sql .= " VALUES (" . $this->db->idate($now) . "," . $user->id . ",'" . $this->numero . "','" . $this->label . "')"; - + $sql .= " VALUES ('" . $this->db->idate($now) . "'," . $user->id . ",'" . $this->numero . "','" . $this->label . "')"; + dol_syslog(get_class($this) . "::create sql=" . $sql, LOG_DEBUG); $resql = $this->db->query($sql); if ($resql) { $id = $this->db->last_insert_id(MAIN_DB_PREFIX . "accounting_system"); - + if ($id > 0) { $this->rowid = $id; $result = $this->rowid; @@ -127,7 +127,7 @@ class AccountancySystem $this->error = "AccountancySystem::Create Erreur $result"; dol_syslog($this->error, LOG_ERR); } - + return $result; } } \ No newline at end of file diff --git a/htdocs/comm/mailing/class/advtargetemailing.class.php b/htdocs/comm/mailing/class/advtargetemailing.class.php index b0d970248eb3a1aba7625ab607251aab5e39dae1..a0744fcfa04145ae81418309e1b6beac2ecfd6cb 100644 --- a/htdocs/comm/mailing/class/advtargetemailing.class.php +++ b/htdocs/comm/mailing/class/advtargetemailing.class.php @@ -124,7 +124,7 @@ class AdvanceTargetingMailing extends CommonObject $sql.= " ".(! isset($this->fk_mailing)?'NULL':"'".$this->fk_mailing."'").","; $sql.= " ".(! isset($this->filtervalue)?'NULL':"'".$this->db->escape($this->filtervalue)."'").","; $sql.= " ".$user->id.","; - $sql.= " ".$this->db->idate(dol_now()).","; + $sql.= " '".$this->db->idate(dol_now())."',"; $sql.= " ".$user->id; diff --git a/htdocs/compta/bank/class/paymentvarious.class.php b/htdocs/compta/bank/class/paymentvarious.class.php index 9812269dcb69a1364148554e2c3e10e07fc4ed81..1d0568c8064e35767cf310616403179a8cf41c15 100644 --- a/htdocs/compta/bank/class/paymentvarious.class.php +++ b/htdocs/compta/bank/class/paymentvarious.class.php @@ -86,9 +86,9 @@ class PaymentVarious extends CommonObject // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."payment_various SET"; - $sql.= " tms=".$this->db->idate($this->tms).","; - $sql.= " datep=".$this->db->idate($this->datep).","; - $sql.= " datev=".$this->db->idate($this->datev).","; + $sql.= " tms='".$this->db->idate($this->tms)."',"; + $sql.= " datep='".$this->db->idate($this->datep)."',"; + $sql.= " datev='".$this->db->idate($this->datev)."',"; $sql.= " sens=".$this->sens.","; $sql.= " amount=".price2num($this->amount).","; $sql.= " fk_typepayment=".$this->fk_typepayment."',"; diff --git a/htdocs/compta/localtax/class/localtax.class.php b/htdocs/compta/localtax/class/localtax.class.php index e59b2c6ce2b88ad3f6ef0519b030e54ecfc1ca95..870c123b4689bdf9280cdd03b1c4a45121abc13f 100644 --- a/htdocs/compta/localtax/class/localtax.class.php +++ b/htdocs/compta/localtax/class/localtax.class.php @@ -151,9 +151,9 @@ class Localtax extends CommonObject // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."localtax SET"; $sql.= " localtaxtype=".$this->ltt.","; - $sql.= " tms=".$this->db->idate($this->tms).","; - $sql.= " datep=".$this->db->idate($this->datep).","; - $sql.= " datev=".$this->db->idate($this->datev).","; + $sql.= " tms='".$this->db->idate($this->tms)."',"; + $sql.= " datep='".$this->db->idate($this->datep)."',"; + $sql.= " datev='".$this->db->idate($this->datev)."',"; $sql.= " amount=".price2num($this->amount).","; $sql.= " label='".$this->db->escape($this->label)."',"; $sql.= " note='".$this->db->escape($this->note)."',"; diff --git a/htdocs/compta/salaries/class/paymentsalary.class.php b/htdocs/compta/salaries/class/paymentsalary.class.php index d3f76a84bdfd4f2f5864abb5822aca46fe86be44..ee2832cb08e8efebea107d779011bdd7e35699ce 100644 --- a/htdocs/compta/salaries/class/paymentsalary.class.php +++ b/htdocs/compta/salaries/class/paymentsalary.class.php @@ -34,7 +34,7 @@ class PaymentSalary extends CommonObject //public $element='payment_salary'; //!< Id that identify managed objects //public $table_element='payment_salary'; //!< Name of table without prefix where object is stored public $picto='payment'; - + public $tms; public $fk_user; public $datep; @@ -97,16 +97,16 @@ class PaymentSalary extends CommonObject // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."payment_salary SET"; - $sql.= " tms=".$this->db->idate($this->tms).","; + $sql.= " tms='".$this->db->idate($this->tms)."',"; $sql.= " fk_user=".$this->fk_user.","; - $sql.= " datep=".$this->db->idate($this->datep).","; - $sql.= " datev=".$this->db->idate($this->datev).","; + $sql.= " datep='".$this->db->idate($this->datep)."',"; + $sql.= " datev='".$this->db->idate($this->datev)."',"; $sql.= " amount=".price2num($this->amount).","; $sql.= " fk_typepayment=".$this->fk_typepayment."',"; $sql.= " num_payment='".$this->db->escape($this->num_payment)."',"; $sql.= " label='".$this->db->escape($this->label)."',"; - $sql.= " datesp=".$this->db->idate($this->datesp).","; - $sql.= " dateep=".$this->db->idate($this->dateep).","; + $sql.= " datesp='".$this->db->idate($this->datesp)."',"; + $sql.= " dateep='".$this->db->idate($this->dateep)."',"; $sql.= " note='".$this->db->escape($this->note)."',"; $sql.= " fk_bank=".($this->fk_bank > 0 ? "'".$this->fk_bank."'":"null").","; $sql.= " fk_user_author=".$this->fk_user_author.","; @@ -548,7 +548,7 @@ class PaymentSalary extends CommonObject } } - + /** * Retourne le libelle du statut d'une facture (brouillon, validee, abandonnee, payee) * @@ -559,7 +559,7 @@ class PaymentSalary extends CommonObject { return $this->LibStatut($this->statut,$mode); } - + /** * Renvoi le libelle d'un statut donne * @@ -570,7 +570,7 @@ class PaymentSalary extends CommonObject function LibStatut($status,$mode=0) { global $langs; // TODO Renvoyer le libelle anglais et faire traduction a affichage - + $langs->load('compta'); /*if ($mode == 0) { @@ -609,5 +609,5 @@ class PaymentSalary extends CommonObject }*/ return ''; } - + } diff --git a/htdocs/compta/tva/class/tva.class.php b/htdocs/compta/tva/class/tva.class.php index ff26347576e554beef2f4cc86361442b51c6b40c..df1bc8c03cfc260bd002e3c994c37cc2d93d0864 100644 --- a/htdocs/compta/tva/class/tva.class.php +++ b/htdocs/compta/tva/class/tva.class.php @@ -173,9 +173,9 @@ class Tva extends CommonObject // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."tva SET"; - $sql.= " tms=".$this->db->idate($this->tms).","; - $sql.= " datep=".$this->db->idate($this->datep).","; - $sql.= " datev=".$this->db->idate($this->datev).","; + $sql.= " tms='".$this->db->idate($this->tms)."',"; + $sql.= " datep='".$this->db->idate($this->datep)."',"; + $sql.= " datev='".$this->db->idate($this->datev)."',"; $sql.= " amount=".price2num($this->amount).","; $sql.= " label='".$this->db->escape($this->label)."',"; $sql.= " note='".$this->db->escape($this->note)."',"; diff --git a/htdocs/core/class/events.class.php b/htdocs/core/class/events.class.php index 2cfedfb7d1a80b36ba7a0aef8ab508585c4ddbb1..68f033870c2459949b213a6483a17a3d9ea7fad0 100644 --- a/htdocs/core/class/events.class.php +++ b/htdocs/core/class/events.class.php @@ -172,7 +172,7 @@ class Events // extends CommonObject // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."events SET"; $sql.= " type='".$this->db->escape($this->type)."',"; - $sql.= " dateevent=".$this->db->idate($this->dateevent).","; + $sql.= " dateevent='".$this->db->idate($this->dateevent)."',"; $sql.= " description='".$this->db->escape($this->description)."'"; $sql.= " WHERE rowid=".$this->id; diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 5df31da812cc9eabddb9f7cafa30a30e1bd068e7..3460bb7f7111e8a1845f79e56229470ed6aeeee2 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -155,6 +155,25 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase print 'Check php file '.$file['fullname']."\n"; $filecontent=file_get_contents($file['fullname']); + + $ok=true; + $matches=array(); + // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. + preg_match_all('/(..)\s*\.\s*\$this->db->idate\(/', $filecontent, $matches, PREG_SET_ORDER); + foreach($matches as $key => $val) + { + if ($val[1] != '\'"' && $val[1] != '\'\'') + { + $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 a $this->db->idate to forge a sql request without quotes around this date field '.$file['fullname'].' :: '.$val[0]); + //exit; + + $ok=true; $matches=array(); // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. @@ -172,6 +191,7 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase $this->assertTrue($ok, 'Found non escaped string in building of a sql request '.$file['fullname'].' ('.$val[0].'). Bad.'); //exit; + // Test that output of $_SERVER\[\'QUERY_STRING\'\] is escaped. $ok=true; $matches=array();