From f2f1f5eeddba6c782e52cad322ffe3600adbbe74 Mon Sep 17 00:00:00 2001 From: "Laurent Destailleur (aka Eldy)" Date: Thu, 13 Feb 2025 10:28:19 +0100 Subject: [PATCH 1/5] Fix phpunit --- htdocs/compta/facture/class/facture.class.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php index 163e1903e71..ed1a480794d 100644 --- a/htdocs/compta/facture/class/facture.class.php +++ b/htdocs/compta/facture/class/facture.class.php @@ -2496,13 +2496,12 @@ class Facture extends CommonInvoice if (isset($this->retained_warranty)) { $this->retained_warranty = (float) $this->retained_warranty; } - if (!isset($this->fk_user_author) && isset($this->user_author) ) { - $this->fk_user_author = $this->user_author; + if (!isset($this->user_creation_id) && isset($this->fk_user_author) ) { + $this->user_creation_id = $this->fk_user_author; + } + if (!isset($this->user_validation_id) && isset($this->fk_user_valid) ) { + $this->user_validation_id = $this->fk_user_valid; } - - - // Check parameters - // Put here code to add control on parameters values // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."facture SET"; @@ -2527,8 +2526,8 @@ class Facture extends CommonInvoice $sql .= " total_ttc=".(isset($this->total_ttc) ? (float) $this->total_ttc : "null").","; $sql .= " revenuestamp=".((isset($this->revenuestamp) && $this->revenuestamp != '') ? (float) $this->revenuestamp : "null").","; $sql .= " fk_statut=".(isset($this->status) ? (int) $this->status : "null").","; - $sql .= " fk_user_author=".(isset($this->fk_user_author) ? ((int) $this->fk_user_author) : "null").","; - $sql .= " fk_user_valid=".(isset($this->fk_user_valid) ? (int) $this->fk_user_valid : "null").","; + $sql .= " fk_user_author=".(isset($this->user_creation_id) ? ((int) $this->user_creation_id) : "null").","; + $sql .= " fk_user_valid=".(isset($this->user_validation_id) ? (int) $this->user_validation_id : "null").","; $sql .= " fk_facture_source=".(isset($this->fk_facture_source) ? (int) $this->fk_facture_source : "null").","; $sql .= " fk_projet=".(isset($this->fk_project) ? (int) $this->fk_project : "null").","; $sql .= " fk_cond_reglement=".(isset($this->cond_reglement_id) ? (int) $this->cond_reglement_id : "null").","; From e0370a514369a5480198f0ec15f49e3084817bc2 Mon Sep 17 00:00:00 2001 From: Regis Houssin Date: Thu, 13 Feb 2025 11:00:22 +0100 Subject: [PATCH 2/5] FIX sql "order by" is defined twice --- htdocs/contrat/services_list.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/htdocs/contrat/services_list.php b/htdocs/contrat/services_list.php index 4a2fa3fb219..2a871d7a486 100644 --- a/htdocs/contrat/services_list.php +++ b/htdocs/contrat/services_list.php @@ -395,10 +395,6 @@ $parameters = array(); $reshook = $hookmanager->executeHooks('printFieldListWhere', $parameters, $object, $action); // Note that $action and $object may have been modified by hook $sql .= $hookmanager->resPrint; -$sql .= $db->order($sortfield, $sortorder); - -//print $sql; - // Count total nb of records $nbtotalofrecords = ''; if (!getDolGlobalInt('MAIN_DISABLE_FULL_SCANLIST')) { @@ -410,7 +406,7 @@ if (!getDolGlobalInt('MAIN_DISABLE_FULL_SCANLIST')) { } } -// Complete request and execute it with limit +// Complete request and execute it with order and limit $sql .= $db->order($sortfield, $sortorder); if ($limit) { $sql .= $db->plimit($limit + 1, $offset); From c55eb6773ceb969a476058c8350b873fcb48e511 Mon Sep 17 00:00:00 2001 From: Regis Houssin Date: Thu, 13 Feb 2025 17:16:35 +0100 Subject: [PATCH 3/5] FIX wrong path for odt models --- htdocs/compta/facture/class/facture.class.php | 3 ++- htdocs/core/modules/modCommande.class.php | 4 ++-- htdocs/core/modules/modFacture.class.php | 4 ++-- htdocs/core/modules/modPropale.class.php | 4 ++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php index ed1a480794d..5c2ae3b38af 100644 --- a/htdocs/compta/facture/class/facture.class.php +++ b/htdocs/compta/facture/class/facture.class.php @@ -5064,11 +5064,12 @@ class Facture extends CommonInvoice //Avoid php warning Warning: mt_rand(): max(0) is smaller than min(1) when no product exists if (empty($num_prods)) { $num_prods = 1; + $prodids[$num_prods] = 1; } // Initialize parameters $this->id = 0; - $this->entity = 1; + $this->entity = $conf->entity; $this->ref = 'SPECIMEN'; $this->specimen = 1; $this->socid = 1; diff --git a/htdocs/core/modules/modCommande.class.php b/htdocs/core/modules/modCommande.class.php index 88fdc6d8abc..276ff239f75 100644 --- a/htdocs/core/modules/modCommande.class.php +++ b/htdocs/core/modules/modCommande.class.php @@ -94,7 +94,7 @@ class modCommande extends DolibarrModules $r++; $this->const[$r][0] = "COMMANDE_ADDON_PDF_ODT_PATH"; $this->const[$r][1] = "chaine"; - $this->const[$r][2] = "DOL_DATA_ROOT/doctemplates/orders"; + $this->const[$r][2] = "DOL_DATA_ROOT".($conf->entity > 1 ? '/'.$conf->entity : '')."/doctemplates/orders"; $this->const[$r][3] = ""; $this->const[$r][4] = 0; @@ -459,7 +459,7 @@ class modCommande extends DolibarrModules //ODT template $src = DOL_DOCUMENT_ROOT.'/install/doctemplates/orders/template_order.odt'; - $dirodt = DOL_DATA_ROOT.'/doctemplates/orders'; + $dirodt = DOL_DATA_ROOT.($conf->entity > 1 ? '/'.$conf->entity : '').'/doctemplates/orders'; $dest = $dirodt.'/template_order.odt'; if (file_exists($src) && !file_exists($dest)) { diff --git a/htdocs/core/modules/modFacture.class.php b/htdocs/core/modules/modFacture.class.php index 16c8e983673..62402977443 100644 --- a/htdocs/core/modules/modFacture.class.php +++ b/htdocs/core/modules/modFacture.class.php @@ -95,7 +95,7 @@ class modFacture extends DolibarrModules $this->const[$r][0] = "FACTURE_ADDON_PDF_ODT_PATH"; $this->const[$r][1] = "chaine"; - $this->const[$r][2] = "DOL_DATA_ROOT/doctemplates/invoices"; + $this->const[$r][2] = "DOL_DATA_ROOT".($conf->entity > 1 ? '/'.$conf->entity : '')."/doctemplates/invoices"; $this->const[$r][3] = ""; $this->const[$r][4] = 0; $r++; @@ -773,7 +773,7 @@ class modFacture extends DolibarrModules //ODT template $src = DOL_DOCUMENT_ROOT.'/install/doctemplates/invoices/template_invoice.odt'; - $dirodt = DOL_DATA_ROOT.'/doctemplates/invoices'; + $dirodt = DOL_DATA_ROOT.($conf->entity > 1 ? '/'.$conf->entity : '').'/doctemplates/invoices'; $dest = $dirodt.'/template_invoice.odt'; if (file_exists($src) && !file_exists($dest)) { diff --git a/htdocs/core/modules/modPropale.class.php b/htdocs/core/modules/modPropale.class.php index 2e99a82acde..8bac74cc3b8 100644 --- a/htdocs/core/modules/modPropale.class.php +++ b/htdocs/core/modules/modPropale.class.php @@ -99,7 +99,7 @@ class modPropale extends DolibarrModules $this->const[$r][0] = "PROPALE_ADDON_PDF_ODT_PATH"; $this->const[$r][1] = "chaine"; - $this->const[$r][2] = "DOL_DATA_ROOT/doctemplates/proposals"; + $this->const[$r][2] = "DOL_DATA_ROOT".($conf->entity > 1 ? '/'.$conf->entity : '')."/doctemplates/proposals"; $this->const[$r][3] = ""; $this->const[$r][4] = 0; $r++; @@ -489,7 +489,7 @@ class modPropale extends DolibarrModules //ODT template $src = DOL_DOCUMENT_ROOT.'/install/doctemplates/proposals/template_proposal.odt'; - $dirodt = DOL_DATA_ROOT.'/doctemplates/proposals'; + $dirodt = DOL_DATA_ROOT.($conf->entity > 1 ? '/'.$conf->entity : '').'/doctemplates/proposals'; $dest = $dirodt.'/template_proposal.odt'; if (file_exists($src) && !file_exists($dest)) { From 36fd5b7b264d09a6ca69e9e3b4bbd9c840caf7fa Mon Sep 17 00:00:00 2001 From: "Laurent Destailleur (aka Eldy)" Date: Thu, 13 Feb 2025 20:29:25 +0100 Subject: [PATCH 4/5] FIX #CVE-2024-34051 --- htdocs/main.inc.php | 8 ++++---- test/phpunit/SecurityTest.php | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index 88264cf918b..991e73c7ec7 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -170,25 +170,25 @@ function testSqlAndScriptInject($val, $type) $inj += preg_match('/=data:/si', $val); // List of dom events is on https://www.w3schools.com/jsref/dom_obj_event.asp and https://developer.mozilla.org/en-US/docs/Web/Events $inj += preg_match('/on(mouse|drag|key|load|touch|pointer|select|transition)[a-z]*\s*=/i', $val); // onmousexxx can be set on img or any html tag like - $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $val); + $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|bounce|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $val); $inj += preg_match('/on(dblclick|drop|durationchange|emptied|end|ended|error|focus|focusin|focusout|formdata|gotpointercapture|hashchange|input|invalid)[a-z]*\s*=/i', $val); $inj += preg_match('/on(lostpointercapture|offline|online|pagehide|pageshow)[a-z]*\s*=/i', $val); $inj += preg_match('/on(paste|pause|play|playing|progress|ratechange|reset|resize|scroll|search|seeked|seeking|show|stalled|start|submit|suspend)[a-z]*\s*=/i', $val); $inj += preg_match('/on(timeupdate|toggle|unload|volumechange|waiting|wheel)[a-z]*\s*=/i', $val); // More not into the previous list - $inj += preg_match('/on(repeat|begin|finish|beforeinput)[a-z]*\s*=/i', $val); + $inj += preg_match('/on(repeat|begin|finish)[a-z]*\s*=/i', $val); // We refuse html into html because some hacks try to obfuscate evil strings by inserting HTML into HTML. Example: error=alert(1) to bypass test on onerror $tmpval = preg_replace('/<[^<]+>/', '', $val); // List of dom events is on https://www.w3schools.com/jsref/dom_obj_event.asp and https://developer.mozilla.org/en-US/docs/Web/Events $inj += preg_match('/on(mouse|drag|key|load|touch|pointer|select|transition)[a-z]*\s*=/i', $tmpval); // onmousexxx can be set on img or any html tag like - $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $tmpval); + $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|bounce|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $tmpval); $inj += preg_match('/on(dblclick|drop|durationchange|emptied|end|ended|error|focus|focusin|focusout|formdata|gotpointercapture|hashchange|input|invalid)[a-z]*\s*=/i', $tmpval); $inj += preg_match('/on(lostpointercapture|offline|online|pagehide|pageshow)[a-z]*\s*=/i', $tmpval); $inj += preg_match('/on(paste|pause|play|playing|progress|ratechange|reset|resize|scroll|search|seeked|seeking|show|stalled|start|submit|suspend)[a-z]*\s*=/i', $tmpval); $inj += preg_match('/on(timeupdate|toggle|unload|volumechange|waiting|wheel)[a-z]*\s*=/i', $tmpval); // More not into the previous list - $inj += preg_match('/on(repeat|begin|finish|beforeinput)[a-z]*\s*=/i', $tmpval); + $inj += preg_match('/on(repeat|begin|finish)[a-z]*\s*=/i', $tmpval); //$inj += preg_match('/on[A-Z][a-z]+\*=/', $val); // To lock event handlers onAbort(), ... $inj += preg_match('/:|:|:/i', $val); // refused string ':' encoded (no reason to have it encoded) to lock 'javascript:...' diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 4fee9d64026..c10a1f5af7c 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -280,6 +280,14 @@ class SecurityTest extends PHPUnit\Framework\TestCase $result=testSqlAndScriptInject($test, 0); $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject aaa7'); + $test=''; + $result=testSqlAndScriptInject($test, 0); + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject onbeforeintput'); + $test=''; + $result=testSqlAndScriptInject($test, 0); + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject onbounce'); + + $test=''; $result=testSqlAndScriptInject($test, 0); $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject bbb'); From 054010f8ecd55d426c2ac1e5cd5a8bb87e2f83cd Mon Sep 17 00:00:00 2001 From: "Laurent Destailleur (aka Eldy)" Date: Thu, 13 Feb 2025 20:34:55 +0100 Subject: [PATCH 5/5] Fix test --- test/phpunit/SecurityTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index c10a1f5af7c..f4b2f3e25a3 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -1149,6 +1149,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase $this->assertEquals('358080.38', $result); global $leftmenu; // Used into strings to eval + $conf->global->MAIN_FEATURES_LEVEL = 1; $leftmenu = 'AAA'; $result=dol_eval('$conf->currency && preg_match(\'/^(AAA|BBB)/\',$leftmenu)', 1, 1, '1'); @@ -1171,7 +1172,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase print "result16 = ".$result."\n"; $this->assertFalse($result); - $string = '(isModEnabled("agenda") || isModEnabled("resource")) && getDolGlobalInt("MAIN_FEATURES_LEVEL") >= 0 && preg_match(\'/^(admintools|all|XXX)/\', $leftmenu)'; + $string = '(isModEnabled("user") || isModEnabled("resource")) && getDolGlobalInt("MAIN_FEATURES_LEVEL") >= 0 && preg_match(\'/^(admintools|all|XXX)/\', $leftmenu)'; $result=dol_eval($string, 1, 1, '1'); print "result17 = ".$result."\n"; $this->assertTrue($result);