diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 68c8775fd81..7fd4a416790 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 1aac46933f2..8f8f8c67ca8 100644 --- a/htdocs/user/class/user.class.php +++ b/htdocs/user/class/user.class.php @@ -2398,53 +2398,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) { @@ -2508,12 +2509,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 8709c66f15d..8cfa5d10db5 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -186,7 +186,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; @@ -275,6 +279,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'); } /** @@ -299,7 +307,9 @@ class SecurityTest extends PHPUnit\Framework\TestCase $_GET["param5"]="a_1-b"; $_POST["param6"]="">objnotdefined\''; $_POST["param11"]=' Name '; @@ -363,10 +373,18 @@ class SecurityTest extends PHPUnit\Framework\TestCase 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'); + // With alphanohtml, we must convert the html entities like n and disable all entities + $result=GETPOST("param8a", 'alphanohtml'); print __METHOD__." result=".$result."\n"; - $this->assertEquals("HackerassertEquals("Hackersvg onload='console.log(123)'", $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("param8c", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals($_POST['param8c'], $result, 'Test a string with non closing html tag with alphanohtml'); $result=GETPOST("param9", 'alphanohtml'); print __METHOD__." result=".$result."\n";
'.$langs->trans("LastUsersCreated", min($num, $max)).''.$langs->trans("FullList").'