From caead5de9f4f2f3e4aac87f05c8b06d9495c9e9a Mon Sep 17 00:00:00 2001 From: ldestailleur Date: Thu, 6 Mar 2025 03:29:54 +0100 Subject: [PATCH] Fix protect use of sanitize to make sql injection --- htdocs/compta/facture/list.php | 2 +- htdocs/core/db/DoliDB.class.php | 23 ++++++++++++++++-- htdocs/core/lib/functions.lib.php | 2 +- htdocs/projet/class/taskstats.class.php | 2 +- test/phpunit/FunctionsLibTest.php | 31 +++++++++++++++++++++---- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/htdocs/compta/facture/list.php b/htdocs/compta/facture/list.php index 1ef606d0160..2e4920cd379 100644 --- a/htdocs/compta/facture/list.php +++ b/htdocs/compta/facture/list.php @@ -868,7 +868,7 @@ if ($search_status != '-1' && $search_status != '') { $sql .= " AND f.fk_statut = 3"; // abandoned } } else { - $sql .= " AND f.fk_statut IN (".$db->sanitize($db->escape($search_status)).")"; // When search_status is '1,2' for example + $sql .= " AND f.fk_statut IN (".$db->sanitize($search_status).")"; // When search_status is '1,2' for example } } diff --git a/htdocs/core/db/DoliDB.class.php b/htdocs/core/db/DoliDB.class.php index 63847079776..3a2346c2356 100644 --- a/htdocs/core/db/DoliDB.class.php +++ b/htdocs/core/db/DoliDB.class.php @@ -177,7 +177,8 @@ abstract class DoliDB implements Database * Sanitize a string for SQL forging * * @param string $stringtosanitize String to sanitize - * @param int $allowsimplequote 1=Allow simple quotes in string. When string is used as a list of SQL string ('aa', 'bb', ...) + * @param int $allowsimplequote 1=Allow simple quotes in string around val separated by "," but only when string is used as a list of SQL string "'aa', 'bb', 'cc', ..."). Can be used for IN ... + * 2=Allow all simple quotes. If you use this value, the return MUST be escaped to forge SQL strings. * @param int $allowsequals 1=Allow equals sign * @param int $allowsspace 1=Allow space char * @param int $allowschars 1=Allow a-z chars @@ -185,7 +186,25 @@ abstract class DoliDB implements Database */ public function sanitize($stringtosanitize, $allowsimplequote = 0, $allowsequals = 0, $allowsspace = 0, $allowschars = 1) { - return preg_replace('/[^0-9_\-\.,'.($allowschars ? 'a-z' : '').($allowsequals ? '=' : '').($allowsimplequote ? "\'" : '').($allowsspace ? ' ' : '').']/i', '', $stringtosanitize); + //$result = preg_replace('/[^0-9_\-\.,'.($allowschars ? '\p{L}' : '').($allowsequals ? '=' : '').($allowsimplequote ? "\'" : '').($allowsspace ? ' ' : '').']/ui', '', $stringtosanitize); + $result = preg_replace('/[^0-9_\-\.,'.($allowschars ? 'a-z' : '').($allowsequals ? '=' : '').($allowsimplequote ? "\'" : '').($allowsspace ? ' ' : '').']/i', '', $stringtosanitize); + + if ($allowsimplequote == 1) { + // Remove all quotes that are inside a string and not around + $tmpchars = explode(',', $result); + $newstringarray = array(); + foreach ($tmpchars as $tmpchar) { + $reg = array(); + if (preg_match('/^\'(.*)\'$/', $tmpchar, $reg)) { + $newstringarray[] = "'".str_replace("'", "", $reg[1])."'"; + } else { + $newstringarray[] = str_replace("'", "", $tmpchar); + } + } + $result = join(',', $newstringarray); + } + + return $result; } /** diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index b453be32572..25beda1136f 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -14424,7 +14424,7 @@ function dolForgeSQLCriteriaCallback($matches) $reg = array(); $tmpelem = trim($tmpelem); if (preg_match('/^\'(.*)\'$/', $tmpelem, $reg)) { - $tmpelemarray[$tmpkey] = "'".$db->escape($db->sanitize($reg[1], 1, 1, 1, 1))."'"; + $tmpelemarray[$tmpkey] = "'".$db->escape($db->sanitize($reg[1], 2, 1, 1, 1))."'"; } elseif (ctype_digit((string) $tmpelem)) { // if only 0-9 chars, no . $tmpelemarray[$tmpkey] = (int) $tmpelem; } elseif (is_numeric((string) $tmpelem)) { // it can be a float with a . diff --git a/htdocs/projet/class/taskstats.class.php b/htdocs/projet/class/taskstats.class.php index 99a176c9470..f2f3ee3311e 100644 --- a/htdocs/projet/class/taskstats.class.php +++ b/htdocs/projet/class/taskstats.class.php @@ -162,7 +162,7 @@ class TaskStats extends Stats $sqlwhere[] = " t.datec BETWEEN '".$this->db->idate(dol_get_first_day($this->year, $this->month))."' AND '".$this->db->idate(dol_get_last_day($this->year, $this->month))."'"; } if (!empty($this->priority)) { - $sqlwhere[] = " t.priority IN (".$this->db->sanitize((string) $this->priority, 1).")"; + $sqlwhere[] = " t.priority = ".((int) $this->priority); } if (count($sqlwhere) > 0) { diff --git a/test/phpunit/FunctionsLibTest.php b/test/phpunit/FunctionsLibTest.php index 30fb513b859..7f585eed557 100644 --- a/test/phpunit/FunctionsLibTest.php +++ b/test/phpunit/FunctionsLibTest.php @@ -1925,13 +1925,36 @@ class FunctionsLibTest extends CommonClassTest */ public function testNaturalSearch() { + global $db; + $s = natural_search("t.field", "abc def"); - $this->assertEquals($s, " AND (t.field LIKE '%abc%' AND t.field LIKE '%def%')"); + $this->assertEquals(" AND (t.field LIKE '%abc%' AND t.field LIKE '%def%')", $s); $s = natural_search("t.field", "'abc def' ghi"); - $this->assertEquals($s, " AND (t.field LIKE '%abc def%' AND t.field LIKE '%ghi%')"); + $this->assertEquals(" AND (t.field LIKE '%abc def%' AND t.field LIKE '%ghi%')", $s); - $s = natural_search("t.field", "abc def,ghi", 3); - $this->assertEquals($s, " AND (t.field IN ('abc def','ghi'))"); + $s = natural_search("t.field", "abc def,ghi", 3); // mode 3 is to provide a list of string separated with coma + $this->assertEquals(" AND (t.field IN ('abc def','ghi'))", $s); + + $s = natural_search("t.field", "'ab\'c' def','ghi', 'jkl'", 3); // mode 3 is to provide a list of string separated with coma + $this->assertEquals(" AND (t.field IN ('abc def','ghi','jkl'))", $s); + + $s = natural_search("t.field", "a,b", 3); // mode 3 is to provide a list of string separated with coma + $this->assertEquals(" AND (t.field IN ('a','b'))", $s); + + $s = natural_search("t.field", "A'@%B", 3); // mode 3 is to provide a list of string separated with coma + $this->assertEquals(" AND (t.field IN ('AB'))", $s); + + /* + $s = $db->sanitize("a,b", 1); + var_dump($s); + $s = $db->sanitize("'a',b", 1); + var_dump($s); + $s = $db->sanitize("'a'b',c", 1); + var_dump($s); + */ + + //$s = natural_search("t.field", "KØB", 3); // mode 3 is to provide a list of string separated with coma + //$this->assertEquals(" AND (t.field IN ('KØB'))", $s); } }