lainwir3d/dolibarr
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

FIx #yogosha13798

Browse Source
This commit is contained in:
Laurent Destailleur 2022-12-05 15:05:40 +01:00
parent 7deccc97b1
commit 57371302be
3 changed files with 43 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
htdocs/core/lib/functions.lib.php
View File

@ -6909,7 +6909,6 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1,
$stringtoclean = preg_replace('/:/i', ':', $stringtoclean);
$stringtoclean = preg_replace('/:|&#0+58|&#x3A/i', '', $stringtoclean); // refused string ':' encoded (no reason to have a : encoded like this) to disable 'javascript:...'
$stringtoclean = preg_replace('/javascript\s*:/i', '', $stringtoclean);
$temp = strip_tags($stringtoclean, $allowed_tags_string); // Warning: This remove also undesired </> changing string obfuscated with </> that pass injection detection into harmfull string
@ -6923,7 +6922,7 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1,
// Remove 'javascript:' that we should not find into a text with
// Warning: This is not reliable to fight against obfuscated javascript, there is a lot of other solution to include js into a common html tag (only filtered by a GETPOST(.., powerfullfilter)).
if ($cleanalsojavascript) {
$temp = preg_replace('/javascript\s*:/i', '', $temp);
$temp = preg_replace('/j\s*a\s*v\s*a\s*s\s*c\s*r\s*i\s*p\s*t\s*:/i', '', $temp);
}
$temp = str_replace('__!DOCTYPE_HTML__', '<!DOCTYPE html>', $temp); // Restore the DOCTYPE
@ -7149,6 +7148,9 @@ function dol_htmlwithnojs($stringtoencode, $nouseofiframesandbox = 0, $check = '
}
}
// Clean some html entities that are useless so text is cleaner
$out = preg_replace('/&(tab|newline);/i', ' ', $out);
// Ckeditor use the numeric entitic for apostrophe so we force it to text entity (all other special chars are
// encoded using text entities) so we can then exclude all numeric entities.
$out = preg_replace('/&#39;/i', '&apos;', $out);
@ -7156,24 +7158,24 @@ function dol_htmlwithnojs($stringtoencode, $nouseofiframesandbox = 0, $check = '
// We replace chars from a/A to z/Z encoded with numeric HTML entities with the real char so we won't loose the chars at the next step (preg_replace).
// No need to use a loop here, this step is not to sanitize (this is done at next step, this is to try to save chars, even if they are
// using a non coventionnel way to be encoded, to not have them sanitized just after)
//$out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', 'realCharForNumericEntities', $out);
$out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', function ($m) {
return realCharForNumericEntities($m); }, $out);
$out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', function ($m) {
return realCharForNumericEntities($m); }, $out);
// Now we remove all remaining HTML entities starting with a number. We don't want such entities.
$out = preg_replace('/&#x?[0-9]+/i', '', $out); // For example if we have j&#x61vascript with an entities without the ; to hide the 'a' of 'javascript'.
// Now we remove all remaining HTML entities starting with a number. We don't want such entities.
$out = preg_replace('/&#x?[0-9]+/i', '', $out); // For example if we have j&#x61vascript with an entities without the ; to hide the 'a' of 'javascript'.
$out = dol_string_onlythesehtmltags($out, 0, 1, 1);
// Keep only some html tags and remove also some 'javascript:' strings
$out = dol_string_onlythesehtmltags($out, 0, 1, 1);
// We should also exclude non expected HTML attributes and clean content of some attributes.
// We should also exclude non expected HTML attributes and clean content of some attributes (keep only alt=, title=...).
if (!empty($conf->global->MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES)) {
// Warning, the function may add a LF so we are forced to trim to compare with old $out without having always a difference and an infinit loop.
$out = dol_string_onlythesehtmlattributes($out);
}
// Restore entity &apos; into &#39; (restricthtml is for html content so we can use html entity)
$out = preg_replace('/&apos;/i', "&#39;", $out);
// Restore entity &apos; into &#39; (restricthtml is for html content so we can use html entity)
$out = preg_replace('/&apos;/i', "&#39;", $out);
} while ($oldstringtoclean != $out);
// Check the limit of external links in a Rich text content. We count '<img' and 'url('

22
htdocs/main.inc.php
View File

@ -89,18 +89,22 @@ function testSqlAndScriptInject($val, $type)
// Decode string first because a lot of things are obfuscated by encoding or multiple encoding.
// So <svg o&#110;load='console.log(&quot;123&quot;)' become <svg onload='console.log(&quot;123&quot;)'
// So "&colon;&apos;" become ":'" (due to ENT_HTML5)
// So "&Tab;&NewLine;" become ""
// So "&lpar;&rpar;" become "()"
// Loop to decode until no more things to decode.
//print "before decoding $val\n";
do {
$oldval = $val;
$val = html_entity_decode($val, ENT_QUOTES | ENT_HTML5);
//$val = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', 'realCharForNumericEntities', $val); // Sometimes we have entities without the ; at end so html_entity_decode does not work but entities is still interpreted by browser.
$val = html_entity_decode($val, ENT_QUOTES | ENT_HTML5); // Decode '&colon;', '&apos;', '&Tab;', '&NewLine', ...
// Sometimes we have entities without the ; at end so html_entity_decode does not work but entities is still interpreted by browser.
$val = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', function ($m) {
// Decode '&#110;', ...
return realCharForNumericEntities($m); }, $val);
// We clean html comments because some hacks try to obfuscate evil strings by inserting HTML comments. Example: on<!-- -->error=alert(1)
$val = preg_replace('/<!--[^>]*-->/', '', $val);
$val = preg_replace('/[\r\n]/', '', $val);
$val = preg_replace('/[\r\n\t]/', '', $val);
} while ($oldval != $val);
//print "type = ".$type." after decoding: ".$val."\n";
@ -123,11 +127,11 @@ function testSqlAndScriptInject($val, $type)
// 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);
$inj += preg_match('/create\s+table/i', $val);
$inj += preg_match('/insert\s+into/i', $val);
$inj += preg_match('/select\s+from/i', $val);
$inj += preg_match('/into\s+(outfile|dumpfile)/i', $val);
$inj += preg_match('/delete\s*from/i', $val);
$inj += preg_match('/create\s*table/i', $val);
$inj += preg_match('/insert\s*into/i', $val);
$inj += preg_match('/select\s*from/i', $val);
$inj += preg_match('/into\s*(outfile|dumpfile)/i', $val);
$inj += preg_match('/user\s*\(/i', $val); // avoid to use function user() or mysql_user() that return current database login
$inj += preg_match('/information_schema/i', $val); // avoid to use request that read information_schema database
$inj += preg_match('/<svg/i', $val); // <svg can be allowed in POST
@ -180,7 +184,7 @@ function testSqlAndScriptInject($val, $type)
//$inj += preg_match('/on[A-Z][a-z]+\*=/', $val); // To lock event handlers onAbort(), ...
$inj += preg_match('/&#58;|&#0000058|&#x3A/i', $val); // refused string ':' encoded (no reason to have it encoded) to lock 'javascript:...'
$inj += preg_match('/javascript\s*:/i', $val);
$inj += preg_match('/j\s*a\s*v\s*a\s*s\s*c\s*r\s*i\s*p\s*t\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)
if ($type == 1 || $type == 3) {

22
test/phpunit/SecurityTest.php
View File

@ -221,6 +221,10 @@ class SecurityTest extends PHPUnit\Framework\TestCase
$result=testSqlAndScriptInject($test, 1);
$this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject for SQL2a. Should find an attack on GET param and did not.');
$test = "delete\nfrom";
$result=testSqlAndScriptInject($test, 1);
$this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject for SQL2b. Should find an attack on GET param and did not.');
$test = 'action=update& ... set ... =';
$result=testSqlAndScriptInject($test, 1);
$this->assertEquals(0, $result, 'Error on testSqlAndScriptInject for SQL2b. Should not find an attack on GET param and did.');
@ -332,7 +336,11 @@ class SecurityTest extends PHPUnit\Framework\TestCase
$test="Text with ' encoded with the numeric html entity converted into text entity &#39; (like when submited by CKEditor)";
$result=testSqlAndScriptInject($test, 0); // result must be 0
$this->assertEquals(0, $result, 'Error on testSqlAndScriptInject mmm');
$this->assertEquals(0, $result, 'Error on testSqlAndScriptInject mmm, result should be 0 and is not');
$test ='<a href="j&Tab;a&Tab;v&Tab;asc&NewLine;ri&Tab;pt:&lpar;a&Tab;l&Tab;e&Tab;r&Tab;t&Tab;(document.cookie)&rpar;">XSS</a>';
$result=testSqlAndScriptInject($test, 0);
$this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject nnn, result should be >= 1 and is not');
$test="/dolibarr/htdocs/index.php/".chr('246')."abc"; // Add the char %F6 into the variable
$result=testSqlAndScriptInject($test, 2);
@ -385,9 +393,8 @@ class SecurityTest extends PHPUnit\Framework\TestCase
$_POST["param16"]='<a style="z-index: 1000">abc</a>';
$_POST["param17"]='<span style="background-image: url(logout.php)">abc</span>';
$_POST["param18"]='<span style="background-image: url(...?...action=aaa)">abc</span>';
//$_POST["param13"]='javascript%26colon%26%23x3B%3Balert(1)';
//$_POST["param14"]='javascripT&javascript#x3a alert(1)';
$_POST["param19"]='<a href="j&Tab;a&Tab;v&Tab;asc&NewLine;ri&Tab;pt:&lpar;alert(document.cookie)&rpar;">XSS</a>';
//$_POST["param19"]='<a href="javascript:alert(document.cookie)">XSS</a>';
$result=GETPOST('id', 'int'); // Must return nothing
print __METHOD__." result=".$result."\n";
@ -507,7 +514,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase
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 &#110;)
// Test with restricthtml: we must remove html open/close tag and content but not htmlentities (we can decode html entities for ascii chars like &#110;)
$result=GETPOST("param6", 'restricthtml');
print __METHOD__." result param6=".$result."\n";
@ -541,6 +548,11 @@ class SecurityTest extends PHPUnit\Framework\TestCase
print __METHOD__." result=".$result."\n";
$this->assertEquals("<img onerror=alert(document.domain) src=>0xbeefed", $result, 'Test 15'); // The GETPOST return a harmull string
$result=GETPOST("param19", 'restricthtml');
print __METHOD__." result=".$result."\n";
$this->assertEquals('<a href="&lpar;alert(document.cookie)&rpar;">XSS</a>', $result, 'Test 19');
// Test with restricthtml + MAIN_RESTRICTHTML_ONLY_VALID_HTML to test disabling of bad atrributes
$conf->global->MAIN_RESTRICTHTML_ONLY_VALID_HTML = 1;