Wavyzz/dolibarr
2
0
Fork 1
mirror of https://github.com/Dolibarr/dolibarr.git synced 2025-12-15 22:11:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix possible remote code execution using dol_concatdesc in dol_eval. To

Browse Source 
allow concat char, you can use
MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL='.'
This commit is contained in:
ldestailleur
2025-04-01 13:25:10 +02:00
parent 95befadad9
commit cc8c7b8329
2 changed files with 51 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -10860,7 +10860,7 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1'
if (getDolGlobalString("MAIN_USE_DOL_EVAL_NEW")) {
return dol_eval_new($s);
} else {
return dol_eval_old($s, $returnvalue, $hideerrors, $onlysimplestring);
return dol_eval_standard($s, $returnvalue, $hideerrors, $onlysimplestring);
}
}
@@ -10974,7 +10974,7 @@ function dol_eval_new($s)
'override_function', 'session_id', 'session_create_id', 'session_regenerate_id',
'call_user_func', 'call_user_func_array', // PREVENT calling forbidden functions
'exec', 'passthru', 'shell_exec', 'system', 'proc_open', 'popen',
'dol_eval', 'executeCLI', 'verifCond', // Native Dolibarr functions
'dol_eval', 'dol_eval_new', 'dol_eval_standard', 'dol_contctdesc', 'executeCLI', 'verifCond', 'GETPOST', // Native Dolibarr functions
'create_function', 'assert', 'mb_ereg_replace', 'mb_eregi_replace', // function with eval capabilities
'dol_compress_dir', 'dol_decode', 'dol_delete_file', 'dol_delete_dir', 'dol_delete_dir_recursive', 'dol_copy', 'archiveOrBackupFile', // more dolibarr functions
'fopen', 'file_put_contents', 'fputs', 'fputscsv', 'fwrite', 'fpassthru', 'mkdir', 'rmdir', 'symlink', 'touch', 'unlink', 'umask', // PHP functions related to file operations
@@ -11063,7 +11063,7 @@ function dol_eval_new($s)
* @see verifCond(), checkPHPCode() to see sanitizing rules that should be very close.
* @phan-suppress PhanPluginUnsafeEval
*/
function dol_eval_old($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1')
function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1')
{
// Only this global variables can be read by eval function and returned to caller
global $conf; // Read of const is done with getDolGlobalString() but we need $conf->currency for example
@@ -11093,8 +11093,10 @@ function dol_eval_old($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring =
if ($onlysimplestring == '2') {
$specialcharsallowed .= '<[]';
}
$specialcharsallowedarray = array();
if (getDolGlobalString('MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL')) {
$specialcharsallowed .= getDolGlobalString('MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL');
$specialcharsallowedarray = str_split(getDolGlobalString('MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL'));
}
if (preg_match('/[^a-z0-9\s'.preg_quote($specialcharsallowed, '/').']/i', $s)) {
if ($returnvalue) {
@@ -11182,7 +11184,7 @@ function dol_eval_old($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring =
return '';
}
}
if (preg_match('/[^0-9]+\.[^0-9]+/', $s)) { // We refuse . if not between 2 numbers
if (preg_match('/[^0-9]+\.[^0-9]+/', $s) && !in_array('.', $specialcharsallowedarray)) { // We refuse . if not between 2 numbers
if ($returnvalue) {
return 'Bad string syntax to evaluate (dot char is forbidden): '.$s;
} else {
@@ -11204,7 +11206,7 @@ function dol_eval_old($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring =
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("function", "call_user_func", "call_user_func_array"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("require", "include", "require_once", "include_once"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("exec", "passthru", "shell_exec", "system", "proc_open", "popen"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_eval", "executeCLI", "verifCond", "GETPOST")); // native dolibarr functions
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_eval", "dol_eval_new", "dol_eval_standard", "dol_concatdesc", "executeCLI", "verifCond", "GETPOST")); // native dolibarr functions
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("eval", "create_function", "assert", "mb_ereg_replace")); // function with eval capabilities
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_compress_dir", "dol_decode", "dol_delete_file", "dol_delete_dir", "dol_delete_dir_recursive", "dol_copy", "archiveOrBackupFile")); // more dolibarr functions
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("fopen", "file_put_contents", "fputs", "fputscsv", "fwrite", "fpassthru", "mkdir", "rmdir", "symlink", "touch", "unlink", "umask"));

56
test/phpunit/SecurityTest.php
View File

@@ -637,7 +637,7 @@ class SecurityTest extends CommonClassTest
$result = dol_eval('1==\x01', 1, 0); // Check that we can't make dol_eval on string containing \ char.
print "result0 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result);
$this->assertStringContainsString('Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string)', $result);
$s = '4 < 5';
$result = (string) dol_eval($s, 1, 1, '2');
@@ -681,9 +681,17 @@ class SecurityTest extends CommonClassTest
print "result6 = ".json_encode($result)."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result), 'The string was not detected as evil');
$result = (string) dol_eval('$a=exec("ls");', 1, 1);
$result = (string) dol_eval('instruction;', 1, 1); // ; is not allowed.
print "result7 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string)', $result, 'The string was not detected as evil');
$result = (string) dol_eval('$a=exec("ls")', 1, 1);
print "result7 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate (mode 1, found call of a function or method without using the direct name of the function)', $result, 'The string was not detected as evil');
$result = (string) dol_eval('$a=exec(\'ls\')', 1, 1);
print "result7 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate (mode 1, found call of a function or method without using the direct name of the function)', $result, 'The string was not detected as evil');
$result = (string) dol_eval('$a=exec ("ls")', 1, 1);
print "result8 = ".$result."\n";
@@ -691,11 +699,11 @@ class SecurityTest extends CommonClassTest
$result = (string) dol_eval("strrev('metsys') ('whoami')", 1, 1);
print "result8b = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (mode 1, found call of a function or method without using the direct name of the function)', $result, 'The string was not detected as evil');
$result = (string) dol_eval('$a="test"; $$a;', 1, 0);
print "result9 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string)', $result, 'The string was not detected as evil');
$result = (string) dol_eval('`ls`', 1, 0);
print "result10 = ".$result."\n";
@@ -703,7 +711,7 @@ class SecurityTest extends CommonClassTest
$result = (string) dol_eval("('ex'.'ec')('echo abc')", 1, 0);
print "result11 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (mode 1, found call of a function or method without using the direct name of the function)', $result, 'The string was not detected as evil');
$result = (string) dol_eval("sprintf(\"%s%s\", \"ex\", \"ec\")('echo abc')", 1, 0);
print "result12 = ".$result."\n";
@@ -715,8 +723,7 @@ class SecurityTest extends CommonClassTest
// Must be allowed
global $leftmenu; // Used into strings to eval
$conf->global->MAIN_FEATURES_LEVEL = 1;
global $mainmenu,$leftmenu; // Used into following strings to eval
$leftmenu = 'AAA';
$result = dol_eval('$conf->currency && preg_match(\'/^(AAA|BBB)/\',$leftmenu)', 1, 1, '1');
@@ -750,17 +757,42 @@ class SecurityTest extends CommonClassTest
print "result18 = ".$result."\n";
$this->assertFalse($result);
$mainmenu = 'TTT';
$leftmenu = 'LLL';
$result = (string) dol_eval('$mainmenu=\'T2\' && ($mainmenu == \'TTT\')', 1, 0);
print "result11 = ".$result."\n";
$this->assertEquals('1', $result, 'The string was not detected as evil');
// Test option MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL
$conf->global->MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL = '.';
$mainmenu = 'ex';
$result = (string) dol_eval('$mainmenu.\'ec\'', 1, 0);
print "result11 = ".$result."\n";
$this->assertStringContainsString('exec', $result, 'With MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL=. we should accept concat');
$mainmenu = 'ex';
$leftmenu = 'ec';
$result = (string) dol_eval("\$mainmenu.\$leftmenu", 1, 0);
print "result11 = ".$result."\n";
$this->assertStringContainsString('exec', $result, 'The string was not detected as evil');
$conf->global->MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL = '';
// Not allowed
$a = 'ab';
$result = (string) dol_eval("(\$a.'s')", 1, 0);
$leftmenu = 'ab';
$result = (string) dol_eval("(\$leftmenu.'s')", 1, 0);
print "result19 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 19 - The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (dot char is forbidden)', $result, 'Test 19 - The string was not detected as evil');
$leftmenu = 'abs';
$result = (string) dol_eval('$leftmenu(-5)', 1, 0);
print "result20 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 20 - The string was not detected as evil');
$this->assertStringContainsString('Bad string syntax to evaluate (mode 1, found a call using "$abc(" or "$abc (" instead of using the direct name of the function)', $result, 'Test 20 - The string was not detected as evil');
$result = (string) dol_eval('str_replace("z","e","zxzc")("whoami");', 1, 0);
print "result21 = ".$result."\n";