diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 16861bd5476..d966af8305c 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -11097,10 +11097,8 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr 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) { @@ -11111,6 +11109,17 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr } } + // Check if we found a ? without a space before and after + $tmps = str_replace(' ? ', '__XXX__', $s); + if (strpos($tmps, '?') !== false) { + if ($returnvalue) { + return 'Bad string syntax to evaluate (The char ? can be used only with a space before and after): '.$s; + } else { + dol_syslog('Bad string syntax to evaluate (The char ? can be used only with a space before and after): '.$s, LOG_WARNING); + return ''; + } + } + // Check if there is a < or <= without spaces before/after if (preg_match('/<=?[^\s]/', $s)) { if ($returnvalue) { @@ -11188,12 +11197,16 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr return ''; } } - 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 { - dol_syslog('Bad string syntax to evaluate (dot char is forbidden): '.$s, LOG_WARNING); - return ''; + + // Disallow also concat + if (getDolGlobalString('MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL')) { + if (preg_match('/[^0-9]+\.[^0-9]+/', $s)) { // We refuse . if not between 2 numbers + if ($returnvalue) { + return 'Bad string syntax to evaluate (dot char is forbidden): '.$s; + } else { + dol_syslog('Bad string syntax to evaluate (dot char is forbidden): '.$s, LOG_WARNING); + return ''; + } } } @@ -11203,8 +11216,6 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr // We list all forbidden function as keywords we don't want to see (we don't mind it if is "kewyord(" or just "keyword", we don't want "keyword" at all) $forbiddenphpfunctions = array(); - // @phpcs:ignore - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("base64"."_"."decode", "rawurl"."decode", "url"."decode", "str"."_rot13", "hex"."2bin")); // name of forbidden functions are split to avoid false positive $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("override_function", "session_id", "session_create_id", "session_regenerate_id")); $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("get_defined_functions", "get_defined_vars", "get_defined_constants", "get_declared_classes")); $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("function", "call_user_func", "call_user_func_array")); @@ -11215,6 +11226,11 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr $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")); $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("require", "include")); + if (getDolGlobalString('MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL')) { // We disabllow all function that allow to obfuscate the real name of a function + // @phpcs:ignore + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("base64"."_"."decode", "rawurl"."decode", "url"."decode", "str"."_rot13", "hex"."2bin")); // name of forbidden functions are split to avoid false positive + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_concatdesc")); // native dolibarr functions + } $forbiddenphpmethods = array('invoke', 'invokeArgs'); // Method of ReflectionFunction to execute a function diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 25c0967d967..0216c7249e8 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -713,7 +713,7 @@ class SecurityTest extends CommonClassTest print "result11 = ".$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("('ex'.'ec') /* */ ('ls')", 1, 0); // This will execute exec of ls + $result = (string) dol_eval("('ex'.'ec') /* */ (/* */'ls')", 1, 0); // This will execute exec of ls print "result11 = ".$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'); @@ -768,30 +768,34 @@ class SecurityTest extends CommonClassTest $this->assertEquals('1', $result, 'The string was not detected as evil'); - // Test option MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL + // Test option MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL - $conf->global->MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL = '.'; + $conf->global->MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL = 0; $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'); + print "resultconcat1 = ".$result."\n"; + $this->assertStringContainsString('exec', $result, 'With MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL off. 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'); + print "resultconcat2 = ".$result."\n"; + $this->assertStringContainsString('exec', $result, 'With MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL off. we should accept concat'); - $conf->global->MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL = ''; + // Test option MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL = 1 + + $conf->global->MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL = 1; + + $leftmenu = 'ab'; + $result = (string) dol_eval("(\$leftmenu.'s')", 1, 0); + print "resultconcat3 = ".$result."\n"; + $this->assertStringContainsString('Bad string syntax to evaluate (dot char is forbidden)', $result, 'Test concat - The string was not reported as a bad syntax when it should'); // Not allowed - $leftmenu = 'ab'; - $result = (string) dol_eval("(\$leftmenu.'s')", 1, 0); - print "result19 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate (dot char is forbidden)', $result, 'Test 19 - The string was not detected as evil'); + $conf->global->MAIN_DISALLOW_STRING_OBFUSCATION_IN_DOL_EVAL = 0; $leftmenu = 'abs'; $result = (string) dol_eval('$leftmenu(-5)', 1, 0); @@ -809,6 +813,10 @@ class SecurityTest extends CommonClassTest $result = (string) dol_eval('\'exec\'("aaa")', 1, 0); print "result23 = ".$result."\n"; $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result), 'Test 23 - The string was not detected as evil - Can\'t find the string Bad string syntax when i should'); + + $result = (string) dol_eval('1 + 2 ', 1, 0, '2'); + print "result24 = ".$result."\n"; + $this->assertStringContainsString('Bad string syntax to evaluate (The char ? can be used only with a space before and after)', json_encode($result), 'Test 24 - The string was not detected as evil - Can\'t find the string Bad string syntax when i should'); }