diff --git a/htdocs/admin/fckeditor.php b/htdocs/admin/fckeditor.php index 51247ac0b35..140cd7fcfdf 100644 --- a/htdocs/admin/fckeditor.php +++ b/htdocs/admin/fckeditor.php @@ -110,7 +110,8 @@ if (GETPOST('save', 'alpha')) { $fckeditor_skin = GETPOST('fckeditor_skin', 'alpha'); if (!empty($fckeditor_skin)) { - if (!dolibarr_set_const($db, 'FCKEDITOR_SKIN', $fckeditor_skin, 'chaine', 0, '', $conf->entity)) { + $result = dolibarr_set_const($db, 'FCKEDITOR_SKIN', $fckeditor_skin, 'chaine', 0, '', $conf->entity); + if ($result <= 0) { $error++; } } else { @@ -119,7 +120,8 @@ if (GETPOST('save', 'alpha')) { $fckeditor_test = GETPOST('formtestfield', 'restricthtml'); if (!empty($fckeditor_test)) { - if (!dolibarr_set_const($db, 'FCKEDITOR_TEST', $fckeditor_test, 'chaine', 0, '', $conf->entity)) { + $result = dolibarr_set_const($db, 'FCKEDITOR_TEST', $fckeditor_test, 'chaine', 0, '', $conf->entity); + if ($result <= 0) { $error++; } } else { @@ -129,7 +131,7 @@ if (GETPOST('save', 'alpha')) { if (!$error) { setEventMessages($langs->trans("SetupSaved"), null, 'mesgs'); } else { - setEventMessages($langs->trans("Error"), null, 'errors'); + setEventMessages($langs->trans("Error").' '.$db->lasterror(), null, 'errors'); } } diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index 18f8591032f..5bc86336b81 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); + $val = preg_replace('/[\r\n]/', '', $val); } while ($oldval != $val); - //print "after decoding $val\n"; + //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 + // Example of valid UTF8 chars: + // utf8=utf8mb3: '\x0A', '\x0D', '\x7E' + // utf8=utf8mb3: '\xE0\xA0\x80' + // utf8mb4: '\xF0\x9D\x84\x9E' (but this may be refused by the database insert if pagecode is utf8=utf8mb3) + $newval = preg_replace('/[\x00-\x09\x0B-\x0C\x0E-\x1F\x7F]/u', '', $val); // /u operator makes UTF8 valid characters being ignored so are not included into the replace + // Note that $newval may also be completely empty '' when non valid UTF8 are found. + 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 +179,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 e1180c980ef..58ef5cd17e1 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'); } /**