diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 8251c850ddf..a987cf03e4c 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -634,17 +634,17 @@ function GETPOST($paramname, $check = 'alphanohtml', $method = 0, $filter = null $out = checkVal($out, $check, $filter, $options); } - // Sanitizing for special parameters. There is no reason to allow the backtopage, backtolist or backtourl parameter to contains an external URL. + // Sanitizing for special parameters. + // Note: There is no reason to allow the backtopage, backtolist or backtourl parameter to contains an external URL. if ($paramname == 'backtopage' || $paramname == 'backtolist' || $paramname == 'backtourl') { - $out = str_replace('\\', '/', $out); - $out = str_replace(array(':', ';', '@'), '', $out); - + $out = str_replace('\\', '/', $out); // Can be before the loop because only 1 char is replaced. No risk to get it after other replacements. + $out = str_replace(array(':', ';', '@'), '', $out); // Can be before the loop because only 1 char is replaced. No risk to get it after other replacements. do { $oldstringtoclean = $out; $out = str_ireplace(array('javascript', 'vbscript', '&colon', '&#'), '', $out); } while ($oldstringtoclean != $out); - $out = preg_replace(array('/^[a-z]*\/\/+/i'), '', $out); + $out = preg_replace(array('/^[a-z]*\/\/+/i'), '', $out); // We remove schema*// to remove external URL } // Code for search criteria persistence. @@ -684,7 +684,7 @@ function GETPOSTINT($paramname, $method = 0, $filter = null, $options = null, $n } /** - * Return a value after checking on a rule. + * Return a value after checking on a rule. A sanitization may also have been done. * * @param string $out Value to check/clear. * @param string $check Type of check/sanitizing @@ -777,6 +777,11 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = case 'restricthtml': // Recommended for most html textarea do { $oldstringtoclean = $out; + + // We replace chars encoded with numeric HTML entities with real char (to avoid to have numeric entities used for obfuscation of injections) + $out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+);/i', 'realCharForNumericEntities', $out); + $out = preg_replace('/&#x?[0-9]+/i', '', $out); // For example if we have javascript with an entities without the ; to hide the 'a' of 'javascript'. + $out = dol_string_onlythesehtmltags($out, 0, 1, 1); // We should also exclude non expected attributes @@ -797,7 +802,6 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = } - if (!function_exists('dol_getprefix')) { /** * Return a prefix to use for this Dolibarr instance, for session/cookie names or email id. @@ -9738,8 +9742,8 @@ function dolGetButtonAction($label, $html = '', $actionType = 'default', $url = /** * Add space between dolGetButtonTitle * - * @param string $moreClass more css class label - * @return string html of title separator + * @param string $moreClass more css class label + * @return string html of title separator */ function dolGetButtonTitleSeparator($moreClass = "") { diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index c9a7dd21ccc..7b40647ebcb 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -50,9 +50,33 @@ if (!empty($_SERVER['MAIN_SHOW_TUNING_INFO'])) { } } + +/** + * Return the real char for a numeric entities. + * This function is required by testSqlAndScriptInject(). + * + * @param string $matches String of numeric entity + * @return string New value + */ +function realCharForNumericEntities($matches) +{ + $newstringnumentity = $matches[1]; + + if (preg_match('/^x/i', $newstringnumentity)) { + $newstringnumentity = hexdec(preg_replace('/^x/i', '', $newstringnumentity)); + } + + // The numeric value we don't want as entities + if (($newstringnumentity >= 65 && $newstringnumentity <= 90) || ($newstringnumentity >= 97 && $newstringnumentity <= 122)) { + return chr((int) $newstringnumentity); + } + + return '&#'.$matches[1]; +} + /** * Security: WAF layer for SQL Injection and XSS Injection (scripts) protection (Filters on GET, POST, PHP_SELF). - * Warning: Such a protection can't be enough. It is not reliable as it will alwyas be possible to bypass this. Good protection can + * Warning: Such a protection can't be enough. It is not reliable as it will always be possible to bypass this. Good protection can * only be guaranted by escaping data during output. * * @param string $val Value brut found int $_GET, $_POST or PHP_SELF @@ -61,7 +85,7 @@ if (!empty($_SERVER['MAIN_SHOW_TUNING_INFO'])) { */ function testSqlAndScriptInject($val, $type) { - // Decode string first bcause a lot of things are obfuscated by encoding or multiple encoding. + // Decode string first because a lot of things are obfuscated by encoding or multiple encoding. // So assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject expected 0b'); - // Should detect XSS + + // Should detect attack $expectedresult=1; $_SERVER["PHP_SELF"]='/DIR WITH SPACE/htdocs/admin/index.php/'; $result=testSqlAndScriptInject($_SERVER["PHP_SELF"], 2); $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject for PHP_SELF that should detect XSS'); + $test = 'javascript:'; + $result=testSqlAndScriptInject($test, 0); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject for javascript1. Should find an attack and did not.'); + + $test = 'javascript:'; + $result=testSqlAndScriptInject($test, 0); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject for javascript2. Should find an attack and did not.'); + $test = 'javascript&colon;alert(1)'; $result=testSqlAndScriptInject($test, 0); - $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject expected 1a'); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject for javascript2'); $test=""; $result=testSqlAndScriptInject($test, 0); - $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject aaa'); + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject aaa1'); $test=""; $result=testSqlAndScriptInject($test, 2); @@ -328,9 +337,12 @@ class SecurityTest extends PHPUnit\Framework\TestCase $_POST["param10"]='is_object($object) ? ($object->id < 10 ? round($object->id / 2, 2) : (2 * $user->id) * (int) substr($mysoc->zip, 1, 2)) : \'objnotdefined\''; $_POST["param11"]=' Name '; $_POST["param12"]='aaa'; + $_POST["param13"]='n n > < " XSS'; + $_POST["param13b"]='n n > < " XSS'; //$_POST["param13"]='javascript%26colon%26%23x3B%3Balert(1)'; //$_POST["param14"]='javascripT&javascript#x3a alert(1)'; + $result=GETPOST('id', 'int'); // Must return nothing print __METHOD__." result=".$result."\n"; $this->assertEquals($result, ''); @@ -343,7 +355,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals($result, 333, 'Test on param1 with 3rd param = 2'); - // Test alpha + // Test with alpha $result=GETPOST("param2", 'alpha'); print __METHOD__." result=".$result."\n"; @@ -357,7 +369,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals($result, 'dir'); - // Test aZ09 + // Test with aZ09 $result=GETPOST("param1", 'aZ09'); print __METHOD__." result=".$result."\n"; @@ -379,25 +391,22 @@ class SecurityTest extends PHPUnit\Framework\TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals($_GET["param5"], $result); - $result=GETPOST("param6", 'alpha'); - print __METHOD__." result=".$result."\n"; - $this->assertEquals('>', $result); + // Test with nohtml $result=GETPOST("param6", 'nohtml'); print __METHOD__." result=".$result."\n"; $this->assertEquals('">', $result); - $result=GETPOST("param6b"); + // Test with alpha = alphanohtml. We must convert the html entities like n and disable all entities + + $result=GETPOST("param6", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('>', $result); + + $result=GETPOST("param6b", 'alphanohtml'); print __METHOD__." result=".$result."\n"; $this->assertEquals('abc', $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 and disable all entities $result=GETPOST("param8a", 'alphanohtml'); print __METHOD__." result=".$result."\n"; $this->assertEquals("Hackersvg onload='console.log(123)'", $result); @@ -434,24 +443,39 @@ class SecurityTest extends PHPUnit\Framework\TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals("Name", $result, 'Test an email string with alphanohtml'); + $result=GETPOST("param13", 'alphanohtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('n n > < XSS', $result, 'Test that html entities are decoded with alpha'); + + // Test with alphawithlgt + $result=GETPOST("param11", 'alphawithlgt'); print __METHOD__." result=".$result."\n"; $this->assertEquals(trim($_POST["param11"]), $result, 'Test an email string with alphawithlgt'); + // Test with restricthtml we must remove html open/close tag and content but not htmlentities (we can decode html entities for ascii chars like n) + + $result=GETPOST("param6", 'restricthtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('">', $result); + + $result=GETPOST("param7", 'restricthtml'); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('"c:\this is a path~1\aaan" abcdef', $result); + $result=GETPOST("param12", 'restricthtml'); print __METHOD__." result=".$result."\n"; $this->assertEquals(trim($_POST["param12"]), $result, 'Test a string with DOCTYPE and restricthtml'); - /*$result=GETPOST("param13", 'alphanohtml'); + $result=GETPOST("param13", 'restricthtml'); print __METHOD__." result=".$result."\n"; - $this->assertEquals(trim($_POST["param13"]), $result, 'Test a string and alphanohtml'); + $this->assertEquals('n n > < " XSS', $result, 'Test that HTML entities are decoded with restricthtml, but only for common alpha chars'); - $result=GETPOST("param14", 'alphanohtml'); + $result=GETPOST("param13b", 'restricthtml'); print __METHOD__." result=".$result."\n"; - $this->assertEquals(trim($_POST["param14"]), $result, 'Test a string and alphanohtml'); - */ + $this->assertEquals('n n > < " XSS', $result, 'Test that HTML entities are decoded with restricthtml, but only for common alpha chars'); - // Special test for GETPOST of backtopage or backtolist parameter + // Special test for GETPOST of backtopage, backtolist or backtourl parameter $_POST["backtopage"]='//www.google.com'; $result=GETPOST("backtopage");