From 4912f6a7205865df407b391e939a74e3f4218f1b Mon Sep 17 00:00:00 2001 From: daraelmin Date: Sat, 19 Nov 2022 22:15:58 +0100 Subject: [PATCH 1/6] Fix #22675 - avoid warning when weight is empty --- htdocs/core/lib/functions2.lib.php | 1 + 1 file changed, 1 insertion(+) diff --git a/htdocs/core/lib/functions2.lib.php b/htdocs/core/lib/functions2.lib.php index 9808fbb28b9..6582665ce73 100644 --- a/htdocs/core/lib/functions2.lib.php +++ b/htdocs/core/lib/functions2.lib.php @@ -1755,6 +1755,7 @@ function weight_convert($weight, &$from_unit, $to_unit) * weigh_convert(320, $f, 0) retournera 0.32 * */ + $weight = is_numeric($weight) ? $weight : 0; while ($from_unit <> $to_unit) { if ($from_unit > $to_unit) { $weight = $weight * 10; From 92647d30e74c145837b12da230ce1190e2d7d6ff Mon Sep 17 00:00:00 2001 From: Christian Humpel Date: Mon, 21 Nov 2022 00:07:50 +0100 Subject: [PATCH 2/6] Fix #22768 remove "natural_search" on status and fk_parent_line. --- htdocs/mrp/mo_list.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/htdocs/mrp/mo_list.php b/htdocs/mrp/mo_list.php index e0b9a8f90da..2b0cdaa0c4d 100644 --- a/htdocs/mrp/mo_list.php +++ b/htdocs/mrp/mo_list.php @@ -174,6 +174,9 @@ if (empty($reshook)) { if (GETPOST('button_removefilter_x', 'alpha') || GETPOST('button_removefilter.x', 'alpha') || GETPOST('button_removefilter', 'alpha')) { // All tests are required to be compatible with all browsers foreach ($object->fields as $key => $val) { $search[$key] = ''; + if ($key == 'status'){ + $search[$key] = -1; + } if (preg_match('/^(date|timestamp|datetime)/', $val['type'])) { $search[$key.'_dtstart'] = ''; $search[$key.'_dtend'] = ''; @@ -247,7 +250,7 @@ foreach ($search as $key => $val) { if ($key == 'status' && $search[$key] == -1) { continue; } - if ($key == 'fk_parent_line') { + if ($key == 'fk_parent_line' && $search[$key] != '') { $sql .= natural_search('moparent.ref', $search[$key], 0); continue; } From b5cafeefcf6e1d80fece565767e24edef87fece6 Mon Sep 17 00:00:00 2001 From: stickler-ci Date: Sun, 20 Nov 2022 23:16:19 +0000 Subject: [PATCH 3/6] Fixing style errors. --- htdocs/mrp/mo_list.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/mrp/mo_list.php b/htdocs/mrp/mo_list.php index 2b0cdaa0c4d..eacd731237c 100644 --- a/htdocs/mrp/mo_list.php +++ b/htdocs/mrp/mo_list.php @@ -174,7 +174,7 @@ if (empty($reshook)) { if (GETPOST('button_removefilter_x', 'alpha') || GETPOST('button_removefilter.x', 'alpha') || GETPOST('button_removefilter', 'alpha')) { // All tests are required to be compatible with all browsers foreach ($object->fields as $key => $val) { $search[$key] = ''; - if ($key == 'status'){ + if ($key == 'status') { $search[$key] = -1; } if (preg_match('/^(date|timestamp|datetime)/', $val['type'])) { From 7c1eac9774bd1fed0b7b4594159f2ac2d12a4011 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 21 Nov 2022 03:58:22 +0100 Subject: [PATCH 4/6] Fix sqli ->escape after ->escapeforlike --- htdocs/contact/list.php | 7 +- htdocs/core/lib/website.lib.php | 13 +- .../modules/import/import_csv.modules.php | 4 +- .../modules/import/import_xlsx.modules.php | 4 +- test/phpunit/Website.class.php | 178 ++++++++++++++++++ 5 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 test/phpunit/Website.class.php diff --git a/htdocs/contact/list.php b/htdocs/contact/list.php index 605fafd9e01..d2c8690eb87 100644 --- a/htdocs/contact/list.php +++ b/htdocs/contact/list.php @@ -502,16 +502,17 @@ if (!empty($conf->socialnetworks->enabled)) { if ($value['active'] && strlen($search_[$key])) { $searchkeyinjsonformat = preg_replace('/"$/', '', preg_replace('/^"/', '', json_encode($search_[$key]))); if (in_array($db->type, array('mysql', 'mysqli'))) { - $sql .= " AND p.socialnetworks REGEXP '\"".$db->escapeforlike($db->escape($key))."\":\"[^\"]*".$db->escapeforlike($db->escape($searchkeyinjsonformat))."'"; + $sql .= " AND p.socialnetworks REGEXP '\"".$db->escape($db->escapeforlike($key))."\":\"[^\"]*".$db->escape($db->escapeforlike($searchkeyinjsonformat))."'"; } elseif ($db->type == 'pgsql') { - $sql .= " AND p.socialnetworks ~ '\"".$db->escapeforlike($db->escape($key))."\":\"[^\"]*".$db->escapeforlike($db->escape($searchkeyinjsonformat))."'"; + $sql .= " AND p.socialnetworks ~ '\"".$db->escape($db->escapeforlike($key))."\":\"[^\"]*".$db->escape($db->escapeforlike($searchkeyinjsonformat))."'"; } else { // Works with all database but not reliable because search only for social network code starting with earched value - $sql .= " AND p.socialnetworks LIKE '%\"".$db->escapeforlike($db->escape($key))."\":\"".$db->escapeforlike($db->escape($searchkeyinjsonformat))."%'"; + $sql .= " AND p.socialnetworks LIKE '%\"".$db->escape($db->escapeforlike($key))."\":\"".$db->escape($db->escapeforlike($searchkeyinjsonformat))."%'"; } } } } +//print $sql; if (strlen($search_email)) { $sql .= natural_search('p.email', $search_email); } diff --git a/htdocs/core/lib/website.lib.php b/htdocs/core/lib/website.lib.php index e9ea4dbcf09..af48ec64885 100644 --- a/htdocs/core/lib/website.lib.php +++ b/htdocs/core/lib/website.lib.php @@ -887,7 +887,7 @@ function getSocialNetworkSharingLinks() * @param string $langcode Language code ('' or 'en', 'fr', 'es', ...) * @param array $otherfilters Other filters * @param int $status 0 or 1, or -1 for both - * @return string HTML content + * @return array Array with results of search */ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $sortfield = 'date_creation', $sortorder = 'DESC', $langcode = '', $otherfilters = 'null', $status = 1) { @@ -925,6 +925,8 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so $found = 0; if (!$error && (empty($max) || ($found < $max)) && (preg_match('/meta/', $algo) || preg_match('/content/', $algo))) { + include_once DOL_DOCUMENT_ROOT.'/website/class/websitepage.class.php'; + $sql = 'SELECT wp.rowid FROM '.MAIN_DB_PREFIX.'website_page as wp'; if (is_array($otherfilters) && !empty($otherfilters['category'])) { $sql .= ', '.MAIN_DB_PREFIX.'categorie_website_page as cwp'; @@ -934,7 +936,7 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so $sql .= " AND wp.status = ".((int) $status); } if ($langcode) { - $sql .= " AND wp.lang ='".$db->escape($langcode)."'"; + $sql .= " AND wp.lang = '".$db->escape($langcode)."'"; } if ($type) { $tmparrayoftype = explode(',', $type); @@ -947,11 +949,11 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so $sql .= " AND ("; $searchalgo = ''; if (preg_match('/meta/', $algo)) { - $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.title LIKE '%".$db->escapeforlike($db->escape($searchstring))."%' OR wp.description LIKE '%".$db->escapeforlike($db->escape($searchstring))."%'"; - $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.keywords LIKE '".$db->escapeforlike($db->escape($searchstring)).",%' OR wp.keywords LIKE '% ".$db->escapeforlike($db->escape($searchstring))."%'"; // TODO Use a better way to scan keywords + $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.title LIKE '%".$db->escape($db->escapeforlike($searchstring))."%' OR wp.description LIKE '%".$db->escape($db->escapeforlike($searchstring))."%'"; + $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.keywords LIKE '".$db->escape($db->escapeforlike($searchstring)).",%' OR wp.keywords LIKE '% ".$db->escape($db->escapeforlike($searchstring))."%'"; // TODO Use a better way to scan keywords } if (preg_match('/content/', $algo)) { - $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.content LIKE '%".$db->escapeforlike($db->escape($searchstring))."%'"; + $searchalgo .= ($searchalgo ? ' OR ' : '')."wp.content LIKE '%".$db->escape($db->escapeforlike($searchstring))."%'"; } $sql .= $searchalgo; if (is_array($otherfilters) && !empty($otherfilters['category'])) { @@ -963,6 +965,7 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so //print $sql; $resql = $db->query($sql); + if ($resql) { $i = 0; while (($obj = $db->fetch_object($resql)) && ($i < $max || $max == 0)) { diff --git a/htdocs/core/modules/import/import_csv.modules.php b/htdocs/core/modules/import/import_csv.modules.php index b251d70ad8f..ea1cb757223 100644 --- a/htdocs/core/modules/import/import_csv.modules.php +++ b/htdocs/core/modules/import/import_csv.modules.php @@ -862,8 +862,8 @@ class ImportCsv extends ModeleImports $stringtosearch = json_encode($socialnetwork).':'.json_encode($json->$socialnetwork); //var_dump($stringtosearch); //var_dump($this->db->escape($stringtosearch)); // This provide a value for sql string (but not for a like) - $where[] = $key." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'"; - $filters[] = $col." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'"; + $where[] = $key." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'"; + $filters[] = $col." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'"; //var_dump($where[1]); // This provide a value for sql string inside a like } else { $where[] = $key.' = '.$data[$key]; diff --git a/htdocs/core/modules/import/import_xlsx.modules.php b/htdocs/core/modules/import/import_xlsx.modules.php index 4023c1ecc99..eb781b66586 100644 --- a/htdocs/core/modules/import/import_xlsx.modules.php +++ b/htdocs/core/modules/import/import_xlsx.modules.php @@ -908,8 +908,8 @@ class ImportXlsx extends ModeleImports $stringtosearch = json_encode($socialnetwork).':'.json_encode($json->$socialnetwork); //var_dump($stringtosearch); //var_dump($this->db->escape($stringtosearch)); // This provide a value for sql string (but not for a like) - $where[] = $key." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'"; - $filters[] = $col." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'"; + $where[] = $key." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'"; + $filters[] = $col." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'"; //var_dump($where[1]); // This provide a value for sql string inside a like } else { $where[] = $key.' = '.$data[$key]; diff --git a/test/phpunit/Website.class.php b/test/phpunit/Website.class.php new file mode 100644 index 00000000000..50d0c16453d --- /dev/null +++ b/test/phpunit/Website.class.php @@ -0,0 +1,178 @@ + + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * or see https://www.gnu.org/ + */ + +/** + * \file test/phpunit/WebsiteTest.php + * \ingroup test + * \brief PHPUnit test + * \remarks To run this script as CLI: phpunit filename.php + */ + +global $conf,$user,$langs,$db; +//define('TEST_DB_FORCE_TYPE','mysql'); // This is to force using mysql driver +//require_once 'PHPUnit/Autoload.php'; + +if (! defined('NOREQUIRESOC')) { + define('NOREQUIRESOC', '1'); +} +if (! defined('NOCSRFCHECK')) { + define('NOCSRFCHECK', '1'); +} +if (! defined('NOTOKENRENEWAL')) { + define('NOTOKENRENEWAL', '1'); +} +if (! defined('NOREQUIREMENU')) { + define('NOREQUIREMENU', '1'); // If there is no menu to show +} +if (! defined('NOREQUIREHTML')) { + define('NOREQUIREHTML', '1'); // If we don't need to load the html.form.class.php +} +if (! defined('NOREQUIREAJAX')) { + define('NOREQUIREAJAX', '1'); +} +if (! defined("NOLOGIN")) { + define("NOLOGIN", '1'); // If this page is public (can be called outside logged session) +} +if (! defined("NOSESSION")) { + define("NOSESSION", '1'); +} + +require_once dirname(__FILE__).'/../../htdocs/main.inc.php'; +require_once dirname(__FILE__).'/../../htdocs/core/lib/website.lib.php'; + + +if (empty($user->id)) { + print "Load permissions for admin user nb 1\n"; + $user->fetch(1); + $user->getrights(); +} +$conf->global->MAIN_DISABLE_ALL_MAILS=1; + + +/** + * Class for PHPUnit tests + * + * @backupGlobals disabled + * @backupStaticAttributes enabled + * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. + */ +class WebsiteTest extends PHPUnit\Framework\TestCase +{ + protected $savconf; + protected $savuser; + protected $savlangs; + protected $savdb; + + /** + * Constructor + * We save global variables into local variables + * + * @return SecurityTest + */ + public function __construct() + { + parent::__construct(); + + //$this->sharedFixture + global $conf,$user,$langs,$db; + $this->savconf=$conf; + $this->savuser=$user; + $this->savlangs=$langs; + $this->savdb=$db; + + print __METHOD__." db->type=".$db->type." user->id=".$user->id; + //print " - db ".$db->db; + print "\n"; + } + + /** + * setUpBeforeClass + * + * @return void + */ + public static function setUpBeforeClass() + { + global $conf,$user,$langs,$db; + $db->begin(); // This is to have all actions inside a transaction even if test launched without suite. + + print __METHOD__."\n"; + } + + /** + * tearDownAfterClass + * + * @return void + */ + public static function tearDownAfterClass() + { + global $conf,$user,$langs,$db; + $db->rollback(); + + print __METHOD__."\n"; + } + + /** + * Init phpunit tests + * + * @return void + */ + protected function setUp() + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + print __METHOD__."\n"; + } + + /** + * End phpunit tests + * + * @return void + */ + protected function tearDown() + { + print __METHOD__."\n"; + } + + + /** + * testGetPagesFromSearchCriterias + * + * @return void + */ + public function testGetPagesFromSearchCriterias() + { + global $db; + + $s = "123') OR 1=1-- \' xxx"; + /* + var_dump($s); + var_dump($db->escapeforlike($s)); + var_dump($db->escape($db->escapeforlike($s))); + */ + + $res = getPagesFromSearchCriterias('page,blogpost', 'meta,content', $s, 2, 'date_creation', 'DESC', 'en'); + //var_dump($res); + print __METHOD__." message=".$res['code']."\n"; + // We must found no line (so code should be KO). If we found somethiing, it means there is a SQL injection of the 1=1 + $this->assertEquals($res['code'], 'KO'); + } +} From 322124690c0df1af909b1ac49d627ad63c97b65b Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 21 Nov 2022 14:47:31 +0100 Subject: [PATCH 5/6] Fix sql syntax error --- htdocs/comm/propal/class/propal.class.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/htdocs/comm/propal/class/propal.class.php b/htdocs/comm/propal/class/propal.class.php index c459ea7b84a..3717ed361b8 100644 --- a/htdocs/comm/propal/class/propal.class.php +++ b/htdocs/comm/propal/class/propal.class.php @@ -4418,8 +4418,8 @@ class PropaleLigne extends CommonObjectLine $sql .= ", qty='".price2num($this->qty)."'"; $sql .= ", subprice=".price2num($this->subprice).""; $sql .= ", remise_percent=".price2num($this->remise_percent).""; - $sql .= ", price=".price2num($this->price).""; // TODO A virer - $sql .= ", remise=".price2num($this->remise).""; // TODO A virer + $sql .= ", price=".(float) price2num($this->price).""; // TODO A virer + $sql .= ", remise=".(float) price2num($this->remise).""; // TODO A virer $sql .= ", info_bits='".$this->db->escape($this->info_bits)."'"; if (empty($this->skip_update_total)) { $sql .= ", total_ht=".price2num($this->total_ht).""; From af413cb09f49f4cecc1b6221896bc217184c8b3a Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 21 Nov 2022 15:12:46 +0100 Subject: [PATCH 6/6] Fix #yogosha13195 --- htdocs/comm/action/index.php | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/htdocs/comm/action/index.php b/htdocs/comm/action/index.php index 2afad30c454..e313c1207cb 100644 --- a/htdocs/comm/action/index.php +++ b/htdocs/comm/action/index.php @@ -277,11 +277,11 @@ if (empty($conf->global->AGENDA_DISABLE_EXT)) { // Note: $conf->global->buggedfile can be empty or 'uselocalandtznodaylight' or 'uselocalandtzdaylight' $listofextcals[] = array( 'src' => getDolGlobalString($source), - 'name' => getDolGlobalString($name), - 'offsettz' => (!empty($conf->global->$offsettz) ? $conf->global->$offsettz : 0), - 'color' => getDolGlobalString($color), - 'default' => getDolGlobalString($default), - 'buggedfile' => (isset($conf->global->buggedfile) ? $conf->global->buggedfile : 0) + 'name' => dol_string_nohtmltag(getDolGlobalString($name)), + 'offsettz' => (int) getDolGlobalInt($offsettz, 0), + 'color' => dol_string_nohtmltag(getDolGlobalString($color)), + 'default' => dol_string_nohtmltag(getDolGlobalString($default)), + 'buggedfile' => dol_string_nohtmltag(getDolGlobalString('buggedfile', '')) ); } } @@ -302,11 +302,11 @@ if (empty($user->conf->AGENDA_DISABLE_EXT)) { // Note: $conf->global->buggedfile can be empty or 'uselocalandtznodaylight' or 'uselocalandtzdaylight' $listofextcals[] = array( 'src' => $user->conf->$source, - 'name' => $user->conf->$name, - 'offsettz' => (!empty($user->conf->$offsettz) ? $user->conf->$offsettz : 0), - 'color' => $user->conf->$color, - 'default' => $user->conf->$default, - 'buggedfile' => (isset($user->conf->buggedfile) ? $user->conf->buggedfile : 0) + 'name' => dol_string_nohtmltag($user->conf->$name), + 'offsettz' => (int) (empty($user->conf->$offsettz) ? 0 : $user->conf->$offsettz), + 'color' => dol_string_nohtmltag($user->conf->$color), + 'default' => dol_string_nohtmltag($user->conf->$default), + 'buggedfile' => dol_string_nohtmltag(isset($user->conf->buggedfile) ? $user->conf->buggedfile : '') ); } } @@ -613,7 +613,7 @@ if (!empty($conf->use_javascript_ajax)) { // If javascript on $default = ''; } - $s .= '
 
'; + $s .= '
 
'; } } @@ -636,8 +636,7 @@ if (!empty($conf->use_javascript_ajax)) { // If javascript on if (!preg_match('/showbirthday=/i', $newparam)) { $newparam .= '&showbirthday=1'; } - $link = 'trans("AgendaShowBirthdayEvents");