diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 4a639c69a69..142d91565d6 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -686,7 +686,7 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = $out = dol_string_nohtmltag($out, 0); } break; - case 'alphawithlgt': // No " and no ../ but we keep < > tags. Can be used for email string like "Name " + case 'alphawithlgt': // No " and no ../ but we keep balanced < > tags with no special chars inside. Can be used for email string like "Name " 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 @@ -5762,7 +5762,7 @@ function picto_required() * @param string $stringtoclean String to clean * @param integer $removelinefeed 1=Replace all new lines by 1 space, 0=Only ending new lines are removed others are replaced with \n, 2=Ending new lines are removed but others are kept with a same number of \n than nb of
when there is both "...
\n..." * @param string $pagecodeto Encoding of input/output string - * @param integer $strip_tags 0=Use internal strip, 1=Use strip_tags() php function (bugged when text contains a < char that is not for a html tag) + * @param integer $strip_tags 0=Use internal strip, 1=Use strip_tags() php function (bugged when text contains a < char that is not for a html tag or when tags is not closed like '0000-021 - $temp = preg_replace($pattern, "", $temp); // pass 1 - // $temp after pass 1: 0000-021 - $temp = preg_replace($pattern, "", $temp); // pass 2 - // $temp after pass 2: 0000-021 + $temp = preg_replace($pattern, "", $temp); // pass 1 - $temp after pass 1: 0000-021 + $temp = preg_replace($pattern, "", $temp); // pass 2 - $temp after pass 2: 0000-021 + // removed '<' into non closing html tags like '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 + $inj += preg_match('/onmouse([a-z]*)\s*=/i', $tmpval); // onmousexxx can be set on img or any html tag like + $inj += preg_match('/ondrag([a-z]*)\s*=/i', $tmpval); // + $inj += preg_match('/ontouch([a-z]*)\s*=/i', $tmpval); // + $inj += preg_match('/on(abort|afterprint|beforeprint|beforeunload|blur|canplay|canplaythrough|change|click|contextmenu|copy|cut)\s*=/i', $tmpval); + $inj += preg_match('/on(dblclick|drop|durationchange|ended|error|focus|focusin|focusout|hashchange|input|invalid)\s*=/i', $tmpval); + $inj += preg_match('/on(keydown|keypress|keyup|load|loadeddata|loadedmetadata|loadstart|offline|online|pagehide|pageshow)\s*=/i', $tmpval); + $inj += preg_match('/on(paste|pause|play|playing|progress|ratechange|resize|reset|scroll|search|seeking|select|show|stalled|start|submit|suspend)\s*=/i', $tmpval); + $inj += preg_match('/on(timeupdate|toggle|unload|volumechange|waiting)\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:...' $inj += preg_match('/javascript\s*:/i', $val); diff --git a/htdocs/user/class/user.class.php b/htdocs/user/class/user.class.php index 89c13670890..241057047e5 100644 --- a/htdocs/user/class/user.class.php +++ b/htdocs/user/class/user.class.php @@ -2430,53 +2430,54 @@ class User extends CommonObject $label .= '
'; $label .= img_picto('', $this->picto).' '.$langs->trans("User").''; $label .= ' '.$this->getLibStatut(4); - $label .= '
'.$langs->trans('Name').': '.$this->getFullName($langs, ''); + $label .= '
'.$langs->trans('Name').': '.dol_string_nohtmltag($this->getFullName($langs, '')); if (!empty($this->login)) { - $label .= '
'.$langs->trans('Login').': '.$this->login; + $label .= '
'.$langs->trans('Login').': '.dol_string_nohtmltag($this->login); } if (!empty($this->job)) { - $label .= '
'.$langs->trans("Job").': '.$this->job; + $label .= '
'.$langs->trans("Job").': '.dol_string_nohtmltag($this->job); } - $label .= '
'.$langs->trans("Email").': '.$this->email; + $label .= '
'.$langs->trans("Email").': '.dol_string_nohtmltag($this->email); if (!empty($this->phone)) { - $label .= '
'.$langs->trans("Phone").': '.$this->phone; + $label .= '
'.$langs->trans("Phone").': '.dol_string_nohtmltag($this->phone); } if (!empty($this->admin)) { $label .= '
'.$langs->trans("Administrator").': '.yn($this->admin); } + $company = ''; if (!empty($this->socid)) { // Add thirdparty for external users $thirdpartystatic = new Societe($db); $thirdpartystatic->fetch($this->socid); if (empty($hidethirdpartylogo)) { $companylink = ' '.$thirdpartystatic->getNomUrl(2, (($option == 'nolink') ? 'nolink' : '')); // picto only of company } - $company = ' ('.$langs->trans("Company").': '.$thirdpartystatic->name.')'; + $company = ' ('.$langs->trans("Company").': '.dol_string_nohtmltag($thirdpartystatic->name).')'; } $type = ($this->socid ? $langs->trans("External").$company : $langs->trans("Internal")); - $label .= '
'.$langs->trans("Type").': '.$type; + $label .= '
'.$langs->trans("Type").': '.dol_string_nohtmltag($type); $label .= '
'; if ($infologin > 0) { $label .= '
'; $label .= '
'.$langs->trans("Session").''; - $label .= '
'.$langs->trans("IPAddress").': '.$_SERVER["REMOTE_ADDR"]; + $label .= '
'.$langs->trans("IPAddress").': '.dol_string_nohtmltag(getUserRemoteIP()); if (!empty($conf->global->MAIN_MODULE_MULTICOMPANY)) { - $label .= '
'.$langs->trans("ConnectedOnMultiCompany").': '.$conf->entity.' (user entity '.$this->entity.')'; + $label .= '
'.$langs->trans("ConnectedOnMultiCompany").': '.$conf->entity.' (User entity '.$this->entity.')'; } - $label .= '
'.$langs->trans("AuthenticationMode").': '.$_SESSION["dol_authmode"].(empty($dolibarr_main_demo) ? '' : ' (demo)'); + $label .= '
'.$langs->trans("AuthenticationMode").': '.dol_string_nohtmltag($_SESSION["dol_authmode"].(empty($dolibarr_main_demo) ? '' : ' (demo)')); $label .= '
'.$langs->trans("ConnectedSince").': '.dol_print_date($this->datelastlogin, "dayhour", 'tzuser'); $label .= '
'.$langs->trans("PreviousConnexion").': '.dol_print_date($this->datepreviouslogin, "dayhour", 'tzuser'); - $label .= '
'.$langs->trans("CurrentTheme").': '.$conf->theme; - $label .= '
'.$langs->trans("CurrentMenuManager").': '.$menumanager->name; + $label .= '
'.$langs->trans("CurrentTheme").': '.dol_string_nohtmltag($conf->theme); + $label .= '
'.$langs->trans("CurrentMenuManager").': '.dol_string_nohtmltag($menumanager->name); $s = picto_from_langcode($langs->getDefaultLang()); - $label .= '
'.$langs->trans("CurrentUserLanguage").': '.($s ? $s.' ' : '').$langs->getDefaultLang(); - $label .= '
'.$langs->trans("Browser").': '.$conf->browser->name.($conf->browser->version ? ' '.$conf->browser->version : '').' ('.$_SERVER['HTTP_USER_AGENT'].')'; - $label .= '
'.$langs->trans("Layout").': '.$conf->browser->layout; - $label .= '
'.$langs->trans("Screen").': '.$_SESSION['dol_screenwidth'].' x '.$_SESSION['dol_screenheight']; + $label .= '
'.$langs->trans("CurrentUserLanguage").': '.dol_string_nohtmltag(($s ? $s.' ' : '').$langs->getDefaultLang()); + $label .= '
'.$langs->trans("Browser").': '.dol_string_nohtmltag($conf->browser->name.($conf->browser->version ? ' '.$conf->browser->version : '').' ('.$_SERVER['HTTP_USER_AGENT'].')'); + $label .= '
'.$langs->trans("Layout").': '.dol_string_nohtmltag($conf->browser->layout); + $label .= '
'.$langs->trans("Screen").': '.dol_string_nohtmltag($_SESSION['dol_screenwidth'].' x '.$_SESSION['dol_screenheight']); if ($conf->browser->layout == 'phone') { $label .= '
'.$langs->trans("Phone").': '.$langs->trans("Yes"); } if (!empty($_SESSION["disablemodules"])) { - $label .= '
'.$langs->trans("DisabledModules").':
'.join(', ', explode(',', $_SESSION["disablemodules"])); + $label .= '
'.$langs->trans("DisabledModules").':
'.dol_string_nohtmltag(join(', ', explode(',', $_SESSION["disablemodules"]))); } } if ($infologin < 0) { @@ -2540,12 +2541,12 @@ class User extends CommonObject } if ($withpictoimg > -2 && $withpictoimg != 2) { if (empty($conf->global->MAIN_OPTIMIZEFORTEXTBROWSER)) { - $result .= ''; + $result .= ''; } if ($mode == 'login') { - $result .= dol_trunc($this->login, $maxlen); + $result .= dol_string_nohtmltag(dol_trunc($this->login, $maxlen)); } else { - $result .= $this->getFullName($langs, '', ($mode == 'firstelselast' ? 3 : ($mode == 'firstname' ? 2 : -1)), $maxlen); + $result .= dol_string_nohtmltag($this->getFullName($langs, '', ($mode == 'firstelselast' ? 3 : ($mode == 'firstname' ? 2 : -1)), $maxlen)); } if (empty($conf->global->MAIN_OPTIMIZEFORTEXTBROWSER)) { $result .= ''; diff --git a/htdocs/user/home.php b/htdocs/user/home.php index 4d215685f53..e294be6a219 100644 --- a/htdocs/user/home.php +++ b/htdocs/user/home.php @@ -128,7 +128,7 @@ if ($resql) print ''; print ''; print ''; - print ''; + print ''."\n"; $i = 0; while ($i < $num && $i < $max) diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index c38cf49d018..f485102e06e 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -171,7 +171,11 @@ class SecurityTest extends PHPUnit\Framework\TestCase $_SERVER["PHP_SELF"]='/DIR WITH SPACE/htdocs/admin/index.php?mainmenu=home&leftmenu=setup&username=weservices'; $result=testSqlAndScriptInject($_SERVER["PHP_SELF"], 2); - $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject 1a'); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject expected 0a'); + + $test = 'This is a < inside string with < and > also and tag like before the >'; + $result=testSqlAndScriptInject($test, 0); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject expected 0b'); // Should detect XSS $expectedresult=1; @@ -260,6 +264,10 @@ class SecurityTest extends PHPUnit\Framework\TestCase $test="onerror=alert(1)"; $result=testSqlAndScriptInject($test, 0); $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject jjj'); + + $test="rror=alert(document.location)"; + $result=testSqlAndScriptInject($test, 0); + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject kkk'); } /** @@ -270,106 +278,116 @@ class SecurityTest extends PHPUnit\Framework\TestCase public function testGETPOST() { global $conf,$user,$langs,$db; - $conf=$this->savconf; - $user=$this->savuser; - $langs=$this->savlangs; - $db=$this->savdb; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; - $_COOKIE["id"]=111; - $_GET["param1"]="222"; - $_POST["param1"]="333"; - $_GET["param2"]='a/b#e(pr)qq-rr\cc'; - $_GET["param3"]='"na/b#e(pr)qq-rr\cc'; // Same than param2 + " and n - $_GET["param4"]='../dir'; - $_GET["param5"]="a_1-b"; - $_POST["param6"]="">objnotdefined\''; - $_POST["param11"]=' Name '; + $_COOKIE["id"]=111; + $_GET["param1"]="222"; + $_POST["param1"]="333"; + $_GET["param2"]='a/b#e(pr)qq-rr\cc'; + $_GET["param3"]='"na/b#e(pr)qq-rr\cc'; // Same than param2 + " and n + $_GET["param4"]='../dir'; + $_GET["param5"]="a_1-b"; + $_POST["param6"]="">objnotdefined\''; + $_POST["param11"]=' Name '; - $result=GETPOST('id', 'int'); // Must return nothing - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, ''); + $result=GETPOST('id', 'int'); // Must return nothing + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, ''); - $result=GETPOST("param1", 'int'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, 222, 'Test on param1 with no 3rd param'); + $result=GETPOST("param1", 'int'); + print __METHOD__." result=".$result."\n"; + $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, 'Test on param1 with 3rd param = 2'); + $result=GETPOST("param1", 'int', 2); + print __METHOD__." result=".$result."\n"; + $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"], 'Test on param2'); + // Test alpha + $result=GETPOST("param2", 'alpha'); + print __METHOD__." result=".$result."\n"; + $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', 'Test on param3'); + $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', 'Test on param3'); - $result=GETPOST("param4", 'alpha'); // Must return string sanitized from ../ - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, 'dir'); + $result=GETPOST("param4", 'alpha'); // Must return string sanitized from ../ + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, 'dir'); - // Test aZ09 - $result=GETPOST("param1", 'aZ09'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, $_GET["param1"]); + // Test aZ09 + $result=GETPOST("param1", 'aZ09'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, $_GET["param1"]); - $result=GETPOST("param2", 'aZ09'); // Must return '' as string contains car not in aZ09 definition - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, ''); + $result=GETPOST("param2", 'aZ09'); // Must return '' as string contains car not in aZ09 definition + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, ''); - $result=GETPOST("param3", 'aZ09'); // Must return '' as string contains car not in aZ09 definition - print __METHOD__." result=".$result."\n"; - $this->assertEquals($result, ''); + $result=GETPOST("param3", 'aZ09'); // Must return '' as string contains car not in aZ09 definition + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, ''); - $result=GETPOST("param4", 'aZ09'); // Must return '' as string contains car not in aZ09 definition - print __METHOD__." result=".$result."\n"; - $this->assertEquals('', $result); + $result=GETPOST("param4", 'aZ09'); // Must return '' as string contains car not in aZ09 definition + print __METHOD__." result=".$result."\n"; + $this->assertEquals('', $result); - $result=GETPOST("param5", 'aZ09'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals($_GET["param5"], $result); + $result=GETPOST("param5", 'aZ09'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($_GET["param5"], $result); - $result=GETPOST("param6", 'alpha'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals('>', $result); + $result=GETPOST("param6", 'alpha'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('>', $result); - $result=GETPOST("param6", 'nohtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals('">', $result); + $result=GETPOST("param6", 'nohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('">', $result); - // With restricthtml we must remove html open/close tag and content but not htmlentities like n - $result=GETPOST("param7", 'restricthtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals('"c:\this is a path~1\aaan" abcdef', $result); + // With restricthtml we must remove html open/close tag and content but not htmlentities like n + $result=GETPOST("param7", 'restricthtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('"c:\this is a path~1\aaan" abcdef', $result); - // With alphanohtml, we must convert the html entities like n - $result=GETPOST("param8", 'alphanohtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals("HackerassertEquals("Hackersvg onload='console.log(123)'", $result); - $result=GETPOST("param9", 'alphanohtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals($_POST["param9"], $result); + $result=GETPOST("param8b", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('img src=x onerror=alert(document.location) t=', $result, 'Test a string with non closing html tag with alphanohtml'); - $result=GETPOST("param10", 'alphanohtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals($_POST["param9"], $result, 'We should get param9 after processing param10'); + $result=GETPOST("param8c", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($_POST['param8c'], $result, 'Test a string with non closing html tag with alphanohtml'); - $result=GETPOST("param11", 'alphanohtml'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals("Name", $result, 'Test an email string with alphanohtml'); + $result=GETPOST("param9", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($_POST["param9"], $result); - $result=GETPOST("param11", 'alphawithlgt'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals(trim($_POST["param11"]), $result, 'Test an email string with alphawithlgt'); + $result=GETPOST("param10", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($_POST["param9"], $result, 'We should get param9 after processing param10'); - return $result; + $result=GETPOST("param11", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals("Name", $result, 'Test an email string with alphanohtml'); + + $result=GETPOST("param11", 'alphawithlgt'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals(trim($_POST["param11"]), $result, 'Test an email string with alphawithlgt'); + + return $result; } /**
'.$langs->trans("LastUsersCreated", min($num, $max)).''.$langs->trans("FullList").'