diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index ec067e38c1b..74960833469 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -89,7 +89,7 @@ function testSqlAndScriptInject($val, $type) // Decode string first because a lot of things are obfuscated by encoding or multiple encoding. // So error=alert(1) + $val = preg_replace('//', '', $val); } while ($oldval != $val); - //print "after decoding $val\n"; - - // We clean string because some hacks try to obfuscate evil strings by inserting non printable chars. Example: 'java(ascci09)scr(ascii00)ipt' is processed like 'javascript' (whatever is place of evil ascii char) - // We should use dol_string_nounprintableascii but function is not yet loaded/available - $val = preg_replace('/[\x00-\x1F\x7F]/u', '', $val); // /u operator makes UTF8 valid characters being ignored so are not included into the replace - // We clean html comments because some hacks try to obfuscate evil strings by inserting HTML comments. Example: onerror=alert(1) - $val = preg_replace('//', '', $val); + //print "type = ".$type." after decoding: ".$val."\n"; $inj = 0; + + // We check string because some hacks try to obfuscate evil strings by inserting non printable chars. Example: 'java(ascci09)scr(ascii00)ipt' is processed like 'javascript' (whatever is place of evil ascii char) + // We should use dol_string_nounprintableascii but function is not yet loaded/available + $newval = preg_replace('/[\x00-\x1F\x7F]/u', '', $val); // /u operator makes UTF8 valid characters being ignored so are not included into the replace + if ($newval != $val) { + // If $val has changed after removing non valid UTF8 chars, it means we have an evil string. + $inj += 1; + } + //print 'type='.$type.'-val='.$val.'-newval='.$newval."-inj=".$inj."\n"; + // For SQL Injection (only GET are used to scan for such injection strings) if ($type == 1 || $type == 3) { $inj += preg_match('/delete\s+from/i', $val); @@ -166,7 +172,6 @@ function testSqlAndScriptInject($val, $type) //$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); $inj += preg_match('/vbscript\s*:/i', $val); // For XSS Injection done by adding javascript closing html tags like with onmousemove, etc... (closing a src or href tag with not cleaned param) diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index ccbb0b38ded..fe5cbd01462 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -325,6 +325,11 @@ class SecurityTest extends PHPUnit\Framework\TestCase $test="Text with ' encoded with the numeric html entity converted into text entity ' (like when submited by CKEditor)"; $result=testSqlAndScriptInject($test, 0); // result must be 0 $this->assertEquals(0, $result, 'Error on testSqlAndScriptInject mmm'); + + $test="/dolibarr/htdocs/index.php/".chr('246')."abc"; // Add the char %F6 into the variable + $result=testSqlAndScriptInject($test, 2); + //print "test=".$test." result=".$result."\n"; + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject with a non valid UTF8 char'); } /**