diff --git a/htdocs/accountancy/class/bookkeeping.class.php b/htdocs/accountancy/class/bookkeeping.class.php index 9b84c3a3790..eb17e2728e0 100644 --- a/htdocs/accountancy/class/bookkeeping.class.php +++ b/htdocs/accountancy/class/bookkeeping.class.php @@ -1016,23 +1016,23 @@ class BookKeeping extends CommonObject } elseif ($key == 't.fk_doc' || $key == 't.fk_docdet' || $key == 't.piece_num') { $sqlwhere[] = $this->db->sanitize($key).' = '.((int) $value); } elseif ($key == 't.subledger_account' || $key == 't.numero_compte') { - $sqlwhere[] = $this->db->sanitize($key).' LIKE \''.$this->db->escape($this->db->escapeforlike($value)).'%\''; + $sqlwhere[] = $this->db->sanitize($key)." LIKE '".$this->db->escape($this->db->escapeforlike($value))."%'"; } elseif ($key == 't.date_creation>=') { - $sqlwhere[] = 't.date_creation >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_creation >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_creation<=') { - $sqlwhere[] = 't.date_creation <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_creation <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_export>=') { - $sqlwhere[] = 't.date_export >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_export >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_export<=') { - $sqlwhere[] = 't.date_export <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_export <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_validated>=') { - $sqlwhere[] = 't.date_validated >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_validated >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_validated<=') { - $sqlwhere[] = 't.date_validated <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_validated <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_lim_reglement>=') { - $sqlwhere[] = 't.date_lim_reglement>=\''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_lim_reglement>='".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_lim_reglement<=') { - $sqlwhere[] = 't.date_lim_reglement<=\''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_lim_reglement<='".$this->db->idate((int) $value)."'"; } elseif ($key == 't.credit' || $key == 't.debit') { $sqlwhere[] = natural_search($key, $value, 1, 1); } elseif ($key == 't.reconciled_option') { @@ -1044,7 +1044,7 @@ class BookKeeping extends CommonObject $sqlwhere[] = natural_search("t.code_journal", $value, 3, 1); } } elseif ($key == 't.search_accounting_code_in' && !empty($value)) { - $sqlwhere[] = 't.numero_compte IN ('.$this->db->sanitize($value, 1).')'; + $sqlwhere[] = "t.numero_compte IN (".$this->db->sanitize($value, 1).")"; } else { $sqlwhere[] = natural_search($key, $value, 0, 1); } @@ -1198,7 +1198,7 @@ class BookKeeping extends CommonObject if (count($filter) > 0) { foreach ($filter as $key => $value) { if ($key == 't.doc_date') { - $sqlwhere[] = $this->db->sanitize($key).' = \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = $this->db->sanitize($key)." = '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.doc_date>=') { $sqlwhere[] = "t.doc_date >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.doc_date<=') { @@ -1218,23 +1218,23 @@ class BookKeeping extends CommonObject } elseif ($key == 't.fk_doc' || $key == 't.fk_docdet' || $key == 't.piece_num') { $sqlwhere[] = $this->db->sanitize($key).' = '.((int) $value); } elseif ($key == 't.subledger_account' || $key == 't.numero_compte') { - $sqlwhere[] = $this->db->sanitize($key).' LIKE \''.$this->db->escape($value).'%\''; + $sqlwhere[] = $this->db->sanitize($key)." LIKE '".$this->db->escape($value)."%'"; } elseif ($key == 't.date_creation>=') { - $sqlwhere[] = 't.date_creation >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_creation >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_creation<=') { - $sqlwhere[] = 't.date_creation <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_creation <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.tms>=') { - $sqlwhere[] = 't.tms >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.tms >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.tms<=') { - $sqlwhere[] = 't.tms <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.tms <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_export>=') { - $sqlwhere[] = 't.date_export >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_export >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_export<=') { - $sqlwhere[] = 't.date_export <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_export <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_validated>=') { - $sqlwhere[] = 't.date_validated >= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_validated >= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.date_validated<=') { - $sqlwhere[] = 't.date_validated <= \''.$this->db->idate((int) $value).'\''; + $sqlwhere[] = "t.date_validated <= '".$this->db->idate((int) $value)."'"; } elseif ($key == 't.credit' || $key == 't.debit') { $sqlwhere[] = natural_search($key, $value, 1, 1); } elseif ($key == 't.code_journal' && !empty($value)) { @@ -1251,7 +1251,7 @@ class BookKeeping extends CommonObject } } if (count($sqlwhere) > 0) { - $sql .= ' AND '.implode(" ".$this->db->sanitize($filtermode)." ", $sqlwhere); + $sql .= " AND ".implode(" ".$this->db->sanitize($filtermode)." ", $sqlwhere); } $filter = ''; diff --git a/htdocs/admin/dict.php b/htdocs/admin/dict.php index ae69d06e801..889a87c81fd 100644 --- a/htdocs/admin/dict.php +++ b/htdocs/admin/dict.php @@ -927,6 +927,10 @@ if (empty($reshook)) { $ok = 0; setEventMessages($langs->transnoentities("ErrorCountryCodeMustBe2Char", $langs->transnoentities("Code")), null, 'errors'); } + if ($id == DICT_PAIEMENT && strlen(GETPOST("code")) >= 6) { // 6 char max on code for payment mode codes + $ok = 0; + setEventMessages($langs->transnoentities("ErrorFieldMustHaveLessThanXChar", $langs->transnoentities("Code"), 6), null, 'errors'); + } // Clean some parameters if ((GETPOST("localtax1_type") || (GETPOST('localtax1_type') == '0')) && !GETPOST("localtax1")) { diff --git a/htdocs/comm/mailing/class/advtargetemailing.class.php b/htdocs/comm/mailing/class/advtargetemailing.class.php index 3edc15d62ba..2e7f7ed560c 100644 --- a/htdocs/comm/mailing/class/advtargetemailing.class.php +++ b/htdocs/comm/mailing/class/advtargetemailing.class.php @@ -1013,9 +1013,9 @@ class AdvanceTargetingMailing extends CommonObject */ public function transformToSQL($column_to_test, $criteria) { - $return_sql_criteria = '('; + $return_sql_criteria = "("; - //This is a multiple value test + // This is a multiple value test if (preg_match('/;/', $criteria)) { $return_sql_not_like = array(); $return_sql_like = array(); @@ -1023,23 +1023,23 @@ class AdvanceTargetingMailing extends CommonObject $criteria_array = explode(';', $criteria); foreach ($criteria_array as $inter_criteria) { if (preg_match('/!/', $inter_criteria)) { - $return_sql_not_like[] = '('.$column_to_test.' NOT LIKE \''.str_replace('!', '', $inter_criteria).'\')'; + $return_sql_not_like[] = "(".$this->db->sanitize($column_to_test)." NOT LIKE '".$this->db->sanitize(str_replace('!', '', $inter_criteria))."')"; } else { - $return_sql_like[] = '('.$column_to_test.' LIKE \''.$inter_criteria.'\')'; + $return_sql_like[] = "(".$this->db->sanitize($column_to_test)." LIKE '".$this->db->sanitize($inter_criteria)."')"; } } if (count($return_sql_like) > 0) { - $return_sql_criteria .= '('.implode(' OR ', $return_sql_like).')'; + $return_sql_criteria .= "(".implode(" OR ", $return_sql_like).")"; // element in arrays were sanitized previously } if (count($return_sql_not_like) > 0) { - $return_sql_criteria .= ' AND ('.implode(' AND ', $return_sql_not_like).')'; + $return_sql_criteria .= " AND (".implode(" AND ", $return_sql_not_like).")"; // element in arrays were sanitized previously } } else { - $return_sql_criteria .= $column_to_test.' LIKE \''.$this->db->escape($criteria).'\''; + $return_sql_criteria .= $this->db->sanitize($column_to_test)." LIKE '".$this->db->escape($criteria)."'"; } - $return_sql_criteria .= ')'; + $return_sql_criteria .= ")"; return $return_sql_criteria; } diff --git a/htdocs/core/db/pgsql.class.php b/htdocs/core/db/pgsql.class.php index 5c93b365e44..017d7e63061 100644 --- a/htdocs/core/db/pgsql.class.php +++ b/htdocs/core/db/pgsql.class.php @@ -325,13 +325,13 @@ class DoliDBPgsql extends DoliDB // To have PostgreSQL case sensitive $count_like = 0; - $line = str_replace(' LIKE \'', ' ILIKE \'', $line, $count_like); + $line = str_replace(" LIKE '", " ILIKE '", $line, $count_like); if (getDolGlobalString('PSQL_USE_UNACCENT') && $count_like > 0) { // @see https://docs.PostgreSQL.fr/11/unaccent.html : 'unaccent()' function must be installed before $line = preg_replace('/\s+(\(+\s*)([a-zA-Z0-9\-\_\.]+) ILIKE /', ' \1unaccent(\2) ILIKE ', $line); } - $line = str_replace(' LIKE BINARY \'', ' LIKE \'', $line); + $line = str_replace(" LIKE BINARY '", " LIKE '", $line); // Replace INSERT IGNORE into INSERT $line = preg_replace('/^INSERT IGNORE/', 'INSERT', $line); diff --git a/htdocs/core/db/sqlite3.class.php b/htdocs/core/db/sqlite3.class.php index 3f44a73de5d..82cb3eb2e08 100644 --- a/htdocs/core/db/sqlite3.class.php +++ b/htdocs/core/db/sqlite3.class.php @@ -1300,7 +1300,7 @@ class DoliDBSqlite3 extends DoliDB // FIXME: not for SQLite $fullpathofdump = '/pathtomysqldump/mysqldump'; - $resql = $this->query('SHOW VARIABLES LIKE \'basedir\''); + $resql = $this->query("SHOW VARIABLES LIKE 'basedir'"); if ($resql) { $liste = $this->fetch_array($resql); $basedir = $liste['Value']; @@ -1319,7 +1319,7 @@ class DoliDBSqlite3 extends DoliDB // FIXME: not for SQLite $fullpathofimport = '/pathtomysql/mysql'; - $resql = $this->query('SHOW VARIABLES LIKE \'basedir\''); + $resql = $this->query("SHOW VARIABLES LIKE 'basedir'"); if ($resql) { $liste = $this->fetch_array($resql); $basedir = $liste['Value']; @@ -1386,7 +1386,9 @@ class DoliDBSqlite3 extends DoliDB $result = array(); /* $sql='SHOW STATUS'; - if ($filter) $sql.=" LIKE '".$this->escape($filter)."'"; + if ($filter) { + $sql.=" LIKE '".$this->escape($filter)."'"; + } $resql=$this->query($sql); if ($resql) { diff --git a/htdocs/langs/en_US/errors.lang b/htdocs/langs/en_US/errors.lang index cca1fcf211e..e9286e843f1 100644 --- a/htdocs/langs/en_US/errors.lang +++ b/htdocs/langs/en_US/errors.lang @@ -87,6 +87,7 @@ ErrorBadFormatValueList=The list value cannot have more than one comma: %s%s must not contains special characters. ErrorFieldCanNotContainSpecialNorUpperCharacters=The field %s must not contain special characters, nor upper case characters, and must start with an alphabetical character (a-z) ErrorFieldMustHaveXChar=The field %s must have at least %s characters. +ErrorFieldMustHaveLessThanXChar=The field %s must have a maximum of %s characters. ErrorNoAccountancyModuleLoaded=No accountancy module activated ErrorExportDuplicateProfil=This profile name already exists for this export set. ErrorLDAPSetupNotComplete=Dolibarr-LDAP matching is not complete. diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 38970b7300f..36638715b44 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -293,6 +293,22 @@ class CodingPhpTest extends CommonClassTest //exit; + // Part to scan code vulnerability on SQL injection + + + // Check sql using ' instead of " + $ok = true; + $matches = array(); + preg_match_all('/LIKE \\\/', $filecontent, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { + 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 a LIKE \' when we should have LIKE ' - Bad."); + //exit; + // Check sql string DELETE|OR|AND|WHERE|INSERT ... yyy = ".$xxx // with xxx that is not 'thi' (for $this->db->sanitize) and 'db-' (for $db->sanitize). It means we forget a ' if string, or an (int) if int, when forging sql request.