22 report sec (#39235)
* Sec: sql injection via api in POST of thirdparties/id/banckaccount - reported by MDnyn * Sec: sql injection via api in POST of thirdparties/id/banckaccount - reported by MDnyn --------- Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
This commit is contained in:
parent
118ae2e655
commit
7ec003387a
8 changed files with 51 additions and 20 deletions
|
|
@ -352,7 +352,7 @@ if (getDolGlobalString('MAIN_PRODUCT_PERENTITY_SHARED')) {
|
|||
$sql .= " LEFT JOIN " . MAIN_DB_PREFIX . "accounting_account as aa ON aa.account_number = p." . $db->sanitize($accountancy_field_name) . " AND aa.fk_pcg_version = '" . $db->escape($pcgvercode) . "'";
|
||||
}
|
||||
if (!empty($searchCategoryProductList)) {
|
||||
$sql .= ' LEFT JOIN '.MAIN_DB_PREFIX."categorie_product as cp ON p.rowid = cp.fk_product"; // We'll need this table joined to the select in order to filter by categ
|
||||
$sql .= " LEFT JOIN ".MAIN_DB_PREFIX."categorie_product as cp ON p.rowid = cp.fk_product"; // We'll need this table joined to the select in order to filter by categ
|
||||
}
|
||||
$sql .= ' WHERE p.entity IN ('.getEntity('product').')';
|
||||
if (strlen(trim($search_current_account))) {
|
||||
|
|
|
|||
|
|
@ -518,9 +518,9 @@ class Subscription extends CommonObject
|
|||
*/
|
||||
public function info($id)
|
||||
{
|
||||
$sql = 'SELECT c.rowid, c.datec, c.tms as datem, c.fk_user_creat';
|
||||
$sql .= ' FROM '.MAIN_DB_PREFIX.'subscription as c';
|
||||
$sql .= ' WHERE c.rowid = '.((int) $id);
|
||||
$sql = "SELECT c.rowid, c.datec, c.tms as datem, c.fk_user_creat";
|
||||
$sql .= " FROM ".MAIN_DB_PREFIX."subscription as c";
|
||||
$sql .= " WHERE c.rowid = ".((int) $id);
|
||||
|
||||
$resql = $this->db->query($sql);
|
||||
if ($resql) {
|
||||
|
|
|
|||
|
|
@ -148,7 +148,7 @@ if (GETPOST("delete")) {
|
|||
$obj = $db->fetch_object($resql);
|
||||
|
||||
$sql = "DELETE FROM ".MAIN_DB_PREFIX."boxes";
|
||||
$sql .= " WHERE entity = ".$conf->entity;
|
||||
$sql .= " WHERE entity = ".((int) $conf->entity);
|
||||
$sql .= " AND box_id = ".((int) $obj->rowid);
|
||||
$resql = $db->query($sql);
|
||||
|
||||
|
|
|
|||
|
|
@ -839,7 +839,7 @@ class ContratLigne extends CommonObjectLine
|
|||
if ($this->date_end > 0) {
|
||||
$sql .= ",date_fin_validite";
|
||||
}
|
||||
$sql .= ") VALUES ($this->fk_contrat, '', '".$this->db->escape($this->description)."',";
|
||||
$sql .= ") VALUES (".((int) $this->fk_contrat).", '', '".$this->db->escape($this->description)."',";
|
||||
$sql .= ($this->fk_product > 0 ? $this->fk_product : "null").",";
|
||||
$sql .= " '".$this->db->escape((string) $this->qty)."',";
|
||||
$sql .= " '".$this->db->escape($this->vat_src_code)."',";
|
||||
|
|
|
|||
|
|
@ -349,7 +349,7 @@ class DiscountAbsolute extends CommonObject
|
|||
$sql .= " multicurrency_amount_ht, multicurrency_amount_tva, multicurrency_amount_ttc,";
|
||||
$sql .= " fk_facture_source, fk_invoice_supplier_source, multicurrency_code, multicurrency_tx";
|
||||
$sql .= ")";
|
||||
$sql .= " VALUES (".$conf->entity.", '".$this->db->idate($this->datec != '' ? $this->datec : dol_now())."', ".((int) $this->socid).", ".(empty($this->discount_type) ? 0 : intval($this->discount_type)).", ".((int) $userid).", '".$this->db->escape($this->description)."',";
|
||||
$sql .= " VALUES (".((int) $conf->entity).", '".$this->db->idate($this->datec != '' ? $this->datec : dol_now())."', ".((int) $this->socid).", ".(empty($this->discount_type) ? 0 : intval($this->discount_type)).", ".((int) $userid).", '".$this->db->escape($this->description)."',";
|
||||
$sql .= " ".price2num($this->amount_ht).", ".price2num($this->amount_tva).", ".price2num($this->amount_ttc).", ".price2num($this->tva_tx).", '".$this->db->escape($this->vat_src_code)."',";
|
||||
$sql .= " ".price2num($this->multicurrency_amount_ht).", ".price2num($this->multicurrency_amount_tva).", ".price2num($this->multicurrency_amount_ttc).", ";
|
||||
$sql .= " ".($this->fk_facture_source ? ((int) $this->fk_facture_source) : "null").",";
|
||||
|
|
|
|||
|
|
@ -396,7 +396,7 @@ class CompanyBankAccount extends Account
|
|||
$this->db->begin();
|
||||
|
||||
$sql = "INSERT INTO ".MAIN_DB_PREFIX."societe_rib (fk_soc, type, datec, model_pdf)";
|
||||
$sql .= " VALUES (".((int) $this->socid).", '".$this->type."', '".$this->db->idate($this->datec)."',";
|
||||
$sql .= " VALUES (".((int) $this->socid).", '".$this->db->escape($this->type)."', '".$this->db->idate($this->datec)."',";
|
||||
$sql .= " '".$this->db->escape(getDolGlobalString("BANKADDON_PDF"))."'";
|
||||
$sql .= ")";
|
||||
$resql = $this->db->query($sql);
|
||||
|
|
|
|||
|
|
@ -96,7 +96,7 @@ class UserBankAccount extends Account
|
|||
$now = dol_now();
|
||||
|
||||
$sql = "INSERT INTO ".$this->db->prefix()."user_rib (fk_user, datec)";
|
||||
$sql .= " VALUES (".$this->userid.", '".$this->db->idate($now)."')";
|
||||
$sql .= " VALUES (".((int) $this->userid).", '".$this->db->idate($now)."')";
|
||||
$resql = $this->db->query($sql);
|
||||
if ($resql) {
|
||||
if ($this->db->affected_rows($resql)) {
|
||||
|
|
|
|||
|
|
@ -117,7 +117,7 @@ class CodingPhpTest extends CommonClassTest
|
|||
// To process only 1 file, uncomment this
|
||||
/*
|
||||
foreach($returnlist as $key => $val) {
|
||||
if ($val[0]['name'] != 'societe.class.php') {
|
||||
if ($val[0]['name'] != 'companybankaccount.class.php') {
|
||||
unset($returnlist[$key]);
|
||||
}
|
||||
}
|
||||
|
|
@ -407,21 +407,52 @@ class CodingPhpTest extends CommonClassTest
|
|||
}
|
||||
$this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables in file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELECT ".$myvar...');
|
||||
|
||||
// Check sql string VALUES ... , ".$xxx
|
||||
// Check sql string VALUES ... , ".$xxx or string VALUES ... , '".$xxx
|
||||
// with xxx that is not 'db-' (for $db->escape). It means we forget a ' if string, or an (int) if int, when forging sql request.
|
||||
// ... = " VALUES (".((int) $this->socid).", '".$this->type."', '".$this->db->idate($this->datec)."',";
|
||||
|
||||
$ok = true;
|
||||
$matches = array();
|
||||
preg_match_all('/(VALUES).*,\s*"\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER);
|
||||
preg_match_all('/(VALUES).*,\s*\'?"\s*\.\s*\$([a-z\-\>]+)/', $filecontent, $matches, PREG_SET_ORDER);
|
||||
foreach ($matches as $key => $val) {
|
||||
if ($val[1] == 'VALUES' && $val[2] == 'db-') { // exclude $db->escape(
|
||||
continue;
|
||||
$matches2 = array();
|
||||
preg_match_all('/,\s*\'?"\s*\.\s*\$([a-z\-\>]+)/', $val[0], $matches2, PREG_SET_ORDER);
|
||||
foreach ($matches2 as $key2 => $val2) {
|
||||
if ($val2[1] == 'mydb->escape') { // exclude ".$mydb->escape(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'dbsession->escape') { // exclude ".$dbsession->escape(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'dbsession->idate') { // exclude ".$dbsession->escape(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'this->db->encrypt') { // exclude ".$this->db->encrypt(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'this->db->escape') { // exclude ".$this->db->escape(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'this->db->idate') { // exclude ".$this->db->idate(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'db->encrypt') { // exclude ".$db->encrypt(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'db->escape') { // exclude ".$db->escape(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'db->idate') { // exclude ".$db->idate(
|
||||
continue;
|
||||
}
|
||||
if ($val2[1] == 'this->escape') { // exclude ".$this->db->encrypt(
|
||||
continue;
|
||||
}
|
||||
|
||||
var_dump($matches2);
|
||||
$ok = false;
|
||||
break;
|
||||
}
|
||||
if ($val[1] == 'VALUES' && $val[2] == 'thi' && preg_match('/this->db->encrypt/', $val[0])) { // exclude ".$this->db->encrypt(
|
||||
continue;
|
||||
}
|
||||
var_dump($matches);
|
||||
$ok = false;
|
||||
break;
|
||||
}
|
||||
//print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n";
|
||||
$this->assertTrue($ok, 'Found non quoted or not casted var in sql request '.$file['relativename'].' - Bad.');
|
||||
|
|
|
|||
Loading…
Reference in a new issue