From 92a4314779f7c33994779670820dfe33849b75ba Mon Sep 17 00:00:00 2001 From: ATM john Date: Wed, 6 Jan 2021 10:45:54 +0100 Subject: [PATCH 1/7] Fix security mecanism generating data loss --- htdocs/core/lib/functions.lib.php | 1 + 1 file changed, 1 insertion(+) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index c6fd80f72d0..fccb019a730 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -602,6 +602,7 @@ function GETPOST($paramname, $check = 'none', $method = 0, $filter = null, $opti $out = trim($out); // '"' is dangerous because param in url can close the href= or src= and add javascript functions. // '../' is dangerous because it allows dir transversals + $out = str_replace('"', "''", trim($out)); if (preg_match('/"/', $out)) $out = ''; elseif (preg_match('/\.\.\//', $out)) $out = ''; $out = dol_string_nohtmltag($out); From e44b14364a30f116cdebec4bc2453b096c00163b Mon Sep 17 00:00:00 2001 From: quentin Date: Wed, 6 Jan 2021 14:17:06 +0100 Subject: [PATCH 2/7] FIX useless tracking number displayed on pdf if empty --- htdocs/core/modules/expedition/doc/pdf_rouget.modules.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php b/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php index 506e72ba1b0..7fc3d622254 100644 --- a/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php +++ b/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php @@ -373,14 +373,15 @@ class pdf_rouget extends ModelePdfExpedition $tab_top_alt = $tab_top; $pdf->SetFont('', 'B', $default_font_size - 2); - $pdf->writeHTMLCell(60, 4, $this->posxdesc - 1, $tab_top - 1, $outputlangs->transnoentities("TrackingNumber")." : ".$object->tracking_number, 0, 1, false, true, 'L'); - $tab_top_alt = $pdf->GetY(); //$tab_top_alt += 1; // Tracking number if (!empty($object->tracking_number)) { + $pdf->writeHTMLCell(60, 4, $this->posxdesc - 1, $tab_top - 1, $outputlangs->transnoentities("TrackingNumber")." : ".$object->tracking_number, 0, 1, false, true, 'L'); + $tab_top_alt = $pdf->GetY(); + $object->getUrlTrackingStatus($object->tracking_number); if (!empty($object->tracking_url)) { From ca557cfacd7ba5e4adad85c8ff9254de8020848d Mon Sep 17 00:00:00 2001 From: stickler-ci Date: Wed, 6 Jan 2021 13:22:35 +0000 Subject: [PATCH 3/7] Fixing style errors. --- htdocs/core/modules/expedition/doc/pdf_rouget.modules.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php b/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php index 7fc3d622254..af2cbbca8cc 100644 --- a/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php +++ b/htdocs/core/modules/expedition/doc/pdf_rouget.modules.php @@ -381,7 +381,7 @@ class pdf_rouget extends ModelePdfExpedition { $pdf->writeHTMLCell(60, 4, $this->posxdesc - 1, $tab_top - 1, $outputlangs->transnoentities("TrackingNumber")." : ".$object->tracking_number, 0, 1, false, true, 'L'); $tab_top_alt = $pdf->GetY(); - + $object->getUrlTrackingStatus($object->tracking_number); if (!empty($object->tracking_url)) { From 170814c718cfdad0615753029853f72f6e0442e2 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Wed, 6 Jan 2021 20:21:22 +0100 Subject: [PATCH 4/7] Fix missing rollback --- htdocs/societe/class/societe.class.php | 1 + 1 file changed, 1 insertion(+) diff --git a/htdocs/societe/class/societe.class.php b/htdocs/societe/class/societe.class.php index 2fcfaecb19e..8de2ab23baf 100644 --- a/htdocs/societe/class/societe.class.php +++ b/htdocs/societe/class/societe.class.php @@ -3699,6 +3699,7 @@ class Societe extends CommonObject if ($result < 0) { setEventMessages($this->error, $this->errors, 'errors'); + $this->db->rollback(); return -1; } } From ef8021467b337bd0a117264c3f3c7b14a3ff4ff8 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Wed, 6 Jan 2021 20:41:40 +0100 Subject: [PATCH 5/7] FIX #15892 #15017 --- htdocs/core/lib/functions.lib.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 3bf47b7da7a..054d932a1f1 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -680,7 +680,8 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = if (!is_array($out)) { // '"' is dangerous because param in url can close the href= or src= and add javascript functions. // '../' is dangerous because it allows dir transversals - $out = str_replace(array('"', '"', '../'), '', trim($out)); + $out = str_replace(array('"', '"'), "''", trim($out)); + $out = str_replace(array('../'), '', trim($out)); // keep lines feed $out = dol_string_nohtmltag($out, 0); } From 123bd8172192a6771374178525450c73f167919e Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Wed, 6 Jan 2021 20:42:18 +0100 Subject: [PATCH 6/7] Trim no more required --- htdocs/core/lib/functions.lib.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 054d932a1f1..4ded311f7ed 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -681,7 +681,7 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = // '"' is dangerous because param in url can close the href= or src= and add javascript functions. // '../' is dangerous because it allows dir transversals $out = str_replace(array('"', '"'), "''", trim($out)); - $out = str_replace(array('../'), '', trim($out)); + $out = str_replace(array('../'), '', $out); // keep lines feed $out = dol_string_nohtmltag($out, 0); } From ca11ea98399e74e564ab28ba9a9bbd69476be475 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Wed, 6 Jan 2021 20:47:57 +0100 Subject: [PATCH 7/7] Fix phpunit Signed-off-by: Laurent Destailleur --- test/phpunit/SecurityTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 3eda8354113..d2405e9609d 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -294,20 +294,20 @@ class SecurityTest extends PHPUnit\Framework\TestCase $result=GETPOST("param1", 'int'); print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, 222); + $this->assertEquals($result, 222, 'Test on param1 with no 3rd param'); $result=GETPOST("param1", 'int', 2); print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, 333); + $this->assertEquals($result, 333, 'Test on param1 with 3rd param = 2'); // Test alpha $result=GETPOST("param2", 'alpha'); print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, $_GET["param2"]); + $this->assertEquals($result, $_GET["param2"], 'Test on param2'); $result=GETPOST("param3", 'alpha'); // Must return string sanitized from char " print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, 'na/b#e(pr)qq-rr\cc'); + $this->assertEquals($result, '\'\'na/b#e(pr)qq-rr\cc', 'Test on param3'); $result=GETPOST("param4", 'alpha'); // Must return string sanitized from ../ print __METHOD__." result=".$result."\n"; @@ -346,7 +346,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase // With alphanohtml, we must convert the html entities like n $result=GETPOST("param8", 'alphanohtml'); print __METHOD__." result=".$result."\n"; - $this->assertEquals("HackerassertEquals("Hacker