diff --git a/ChangeLog b/ChangeLog index 4949f5597bd..da9066cc231 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,6 +36,11 @@ The following changes may create regressions for some external modules, but were * Stock movement API GET method output variable names has been harmonized with POST input parameter names * $conf is no more allowed into computed formulae. You ca replace use of $conf->currency by NEW Introduce getCurrency() and $conf->global->xxx by getDolGlobalString('xxx') * Concatenation into computed property of extrafields is off by default. You can enable it with $dolibarr_main_restrict_eval_methods = 'dol_concat' +* Old variable $obj and $object are no more allowed into evaluated strings like computed extrafields or conditions. Use $objectoffield to get current object or $var123 to + instantiate a non existing object. +* The hidden constant MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL has been replaced with the + variable $dolibarr_main_allow_unsecured_special_chars_in_dol_eval into conf.php + ***** ChangeLog for 22.0.3 compared to 22.0.2 ***** diff --git a/htdocs/conf/conf.php.example b/htdocs/conf/conf.php.example index 6cfcee65416..b6a041f1032 100644 --- a/htdocs/conf/conf.php.example +++ b/htdocs/conf/conf.php.example @@ -316,11 +316,11 @@ $dolibarr_main_restrict_os_commands='mariadb-dump, mariadb, mysqldump, mysql, pg // ================================== // A whitelist of functions and methods to restrict the commands you can execute in a custom calculated fields, like "computed fields" of // extrafields or string conditions of extrafields. -// Default value: 'getDolGlobalString,getDolGlobalInt,fetchNoCompute,hasRight,isModEnabled,abs,round' +// Default value: 'getDolGlobalString,getDolGlobalInt,getDolCurrency,fetchNoCompute,hasRight,isModEnabled,isStringVarMatching,abs,round,dol_now,preg_match' // Examples: -// $dolibarr_main_restrict_eval_methods='getDolGlobalString,getDolGlobalInt,fetchNoCompute,hasRight,isModEnabled,abs,round,dol_concat'; +// $dolibarr_main_restrict_eval_methods='getDolGlobalString,getDolGlobalInt,getDolCurrency,fetchNoCompute,hasRight,isModEnabled,isStringVarMatching,abs,min,max,round,dol_now,dol_concat,preg_match'; // -$dolibarr_main_restrict_eval_methods='getDolGlobalString,getDolGlobalInt,fetchNoCompute,hasRight,isModEnabled,abs,round'; +$dolibarr_main_restrict_eval_methods='getDolGlobalString,getDolGlobalInt,getDolCurrency,fetchNoCompute,hasRight,isModEnabled,isStringVarMatching,abs,min,max,round,dol_now,preg_match'; // dolibarr_main_disabled_modules // ================================== diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index a4d8817c8c3..cdf81283cc6 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -11510,13 +11510,13 @@ function dol_getIdFromCode($db, $key, $tablename, $fieldkey = 'code', $fieldid = } /** - * Check if a variable with name $var startx with $text. + * Check if a variable with name $var start with $regextext. * Can be used to forge dol_eval() conditions. * - * @param string $var Variable - * @param string $regextext Text that must be a valid regex string - * @param int<0,1> $matchrule 1=Test if start with, 0=Test if equal - * @return boolean|string True or False, text if bad usage. + * @param string $var Variable + * @param string $regextext Text that must be a valid regex string + * @param int<0,1> $matchrule 1=Test if start with, 0=Test if equal + * @return boolean|string True or False, text if bad usage. */ function isStringVarMatching($var, $regextext, $matchrule = 1) { @@ -11531,7 +11531,7 @@ function isStringVarMatching($var, $regextext, $matchrule = 1) return 'This variable is not accessible with dol_eval'; } } else { - return 'This value for matchrule is not implemented'; + return 'This value '.$matchrule.' for param $matchrule is not yet implemented'; } } @@ -11567,7 +11567,7 @@ function verifCond($strToEvaluate, $onlysimplestring = '1') * @param int<0,1> $returnvalue 0=No return (deprecated, used to execute eval($a=something)). 1=Value of eval is returned (used to eval($something)). * @param int<0,1> $hideerrors 1=Hide errors * @param string $onlysimplestring '0' (deprecated, do not use it anymore) = Accept all chars, - * '1' (most common use) = Accept only simple string with char 'a-z0-9\s^$_+-.*>&|=!?():"\',/@';', + * '1' (most common use, recommended) = Accept only simple string with char 'a-z0-9\s^$_+-.*>&|=!?():"\',/@';', * '2' (used for example for the compute property of extrafields) = Accept also '<[]' * @return string Return result of eval (even if type can be int, it is safer to assume string and find all potential typing issues as abs(dol_eval(...)). * @see verifCond(), checkPHPCode() to see sanitizing rules that should be very close. @@ -11582,7 +11582,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_standard($s, $returnvalue, $hideerrors, $onlysimplestring); + return dol_eval_standard($s, $hideerrors, $onlysimplestring); } } @@ -11863,8 +11863,7 @@ function dol_eval_new($s) * Replace eval function to add more security. * This function is called by dol_eval(), itself called by verifCond() or trans() and transnoentitiesnoconv(). * - * @param string $s String to evaluate - * @param int<0,1> $returnvalue 0=No return (deprecated, used to execute eval($a=something)). 1=Value of eval is returned (used to eval($something)). + * @param string $s String to evaluate with eval($something) * @param int<0,1> $hideerrors 1=Hide errors * @param string $onlysimplestring '0' (deprecated, do not use it anymore)=Accept all chars, * '1' (most common use)=Accept only simple string with char 'a-z0-9\s^$_+-.*>&|=!?():"\',/@';', @@ -11873,89 +11872,89 @@ function dol_eval_new($s) * @see verifCond(), checkPHPCode() to see sanitizing rules that should be very close. * @phan-suppress PhanPluginUnsafeEval */ -function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1') +function dol_eval_standard($s, $hideerrors = 1, $onlysimplestring = '1') { // Only this global variables can be read by eval function and returned to caller // The less we have, the better it is. // $conf is excluded. We can read $conf->global->xxx properties with getDolGlobalString(), $conf->currency with getDolCurrency(), $conf->entity with getDolEntity() - global $db; - global $langs, $user, $website, $websitepage; + global $db, $langs, $user, $website, $websitepage; global $action, $mainmenu, $leftmenu; global $mysoc; global $objectoffield; // To allow the use of $objectoffield in computed fields - // Old variables used (deprecated) - global $object; - global $obj; // To get $obj used into list when dol_eval() is used for computed fields and $obj is not yet $objectoffield + // Old variables (deprecated) + if (getDolGlobalString('MAIN_ALLOW_OLD_VAR_OBJ_IN_DOL_EVAL')) { + global $object; + global $obj; // To get $obj used into list when dol_eval() is used for computed fields and $obj is not yet $objectoffield + } - $isObBufferActive = false; // When true, the ObBuffer must be cleaned in the exception handler - if (!in_array($onlysimplestring, array('0', '1', '2'))) { - return "Bad call of dol_eval. Parameter onlysimplestring must be '0' (deprecated), '1' or '2'"; + $isObBufferActive = false; // When true, the ObBuffer must be cleaned in the exception handler + if ($onlysimplestring == '0') { // '0' is deprecated, we process it as the more secured '1' + $onlysimplestring = '1'; + } + if (!in_array($onlysimplestring, array('1', '2'))) { + return "Bad call of dol_eval. Parameter onlysimplestring must be '1' or '2'"; } try { - // Set $dolibarr_main_restrict_eval_methods_array global $dolibarr_main_restrict_eval_methods; - $dolibarr_main_restrict_eval_methods_array = array(); - if (!empty($dolibarr_main_restrict_eval_methods)) { - $dolibarr_main_restrict_eval_methods_array = explode(',', $dolibarr_main_restrict_eval_methods); + + // Set $dolibarr_main_restrict_eval_methods_array + if (!isset($dolibarr_main_restrict_eval_methods)) { + $dolibarr_main_restrict_eval_methods = 'getDolGlobalString,getDolGlobalInt,getDolCurrency,fetchNoCompute,hasRight,isModEnabled,isStringVarMatching,abs,min,max,round,dol_now,preg_match'; + } + //print '$dolibarr_main_restrict_eval_methods = '.$dolibarr_main_restrict_eval_methods."\n"; + $dolibarr_main_restrict_eval_methods_array = explode(',', $dolibarr_main_restrict_eval_methods); + + if (is_array($s) || $s === 'Array') { + return 'Bad string syntax to evaluate (value is Array): ' . var_export($s, true); } // Test on dangerous char (used for RCE), we allow only characters to make PHP variable testing - if ($onlysimplestring == '1' || $onlysimplestring == '2') { - // We must accept with 1: '1 && getDolGlobalInt("doesnotexist1") && getDolGlobalString("MAIN_FEATURES_LEVEL")' - // We must accept with 1: '$user->hasRight("cabinetmed", "read") && !$object->canvas=="patient@cabinetmed"' - // We must accept with 2: (($reloadedobj = new Task($db)) && ($reloadedobj->fetchNoCompute($object->id) <= 99) && ($secondloadedobj = new Project($db)) && ($secondloadedobj->fetchNoCompute($reloadedobj->fk_project) > 0)) ? $secondloadedobj->ref : "Parent project not found" + // We must accept with 1: '1 && getDolGlobalInt("doesnotexist1") && getDolGlobalString("MAIN_FEATURES_LEVEL")' + // We must accept with 1: '$user->hasRight("cabinetmed", "read") && !$objectoffield->canvas == "patient@cabinetmed"' + // We must accept with 2: (($var1 = new Task($db)) && ($var1->fetchNoCompute($object->id) <= 99) && ($var2 = new Project($db)) && ($var2->fetchNoCompute($var1->fk_project) > 0)) ? $var2->ref : "Parent project not found" - // Check if there is dynamic call (first we check chars are all into a whitelist chars) - $specialcharsallowed = '^$_+-.*>&|=!?():"\',/@'; - if ($onlysimplestring == '2') { - $specialcharsallowed .= '<[]'; - } - if (getDolGlobalString('MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL')) { - $specialcharsallowed .= getDolGlobalString('MAIN_ALLOW_UNSECURED_SPECIAL_CHARS_IN_DOL_EVAL'); - } - if (preg_match('/[^a-z0-9\s' . preg_quote($specialcharsallowed, '/') . ']/i', $s)) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string): ' . $s, LOG_WARNING); - return ''; - } - } + // Check if there is dynamic call (first we check chars are all into a whitelist chars) + $specialcharsallowed = '^$_+-.*>&|=!?():"\',/@'; + if ($onlysimplestring == '2') { + $specialcharsallowed .= '<[]'; // Later we check that < has space before and after + } + global $dolibarr_main_allow_unsecured_special_chars_in_dol_eval; + if (!empty($dolibarr_main_allow_unsecured_special_chars_in_dol_eval)) { + $specialcharsallowed .= (string) $dolibarr_main_allow_unsecured_special_chars_in_dol_eval; + } + if (preg_match('/[^a-z0-9\s' . preg_quote($specialcharsallowed, '/') . ']/i', $s)) { + return 'Bad string syntax to evaluate (found chars that are not chars for a simple one line clean eval string): ' . $s; + } - // 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 we found a | without a space before and after + /* Disabled to allow preg_match('/(AAA|BBB)/') + $tmps = str_replace(' || ', '__XXX__', $s); + if (strpos($tmps, '|') !== false) { + return 'Bad string syntax to evaluate (The char | can be used only when duplicated || with a space before and after): ' . $s; + } + */ - // Check if there is a < or <= without spaces before/after - if (preg_match('/<=?[^\s]/', $s)) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a < or <= without space before and after): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a < or <= without space before and after): ' . $s, LOG_WARNING); - return ''; - } - } + // Check if we found a ? without a space before and after + $tmps = str_replace(' ? ', '__XXX__', $s); + if (strpos($tmps, '?') !== false) { + return 'Bad string syntax to evaluate (The char ? can be used only with a space before and after): ' . $s; + } - // Check if there is dynamic call (first we use black list patterns) - if (preg_match('/\$[\w]*\s*\(/', $s)) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a call using "$abc(" or "$abc (" instead of using the direct name of the function): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a call using "$abc(" or "$abc (" instead of using the direct name of the function): ' . $s, LOG_WARNING); - return ''; - } - } + // Check if there is a < or <= without spaces after + if (preg_match('/<=?[^\s]/', $s)) { + return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a < or <= without space after): ' . $s; + } + + // Check if there is dynamic call (first we use black list patterns) + if (preg_match('/\$[\w]*\s*\(/', $s)) { + return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found a call using "$abc(" or "$abc (" instead of using the direct name of the function): ' . $s; + } + + if (empty($dolibarr_main_restrict_eval_methods)) { + // If $dolibarr_main_restrict_eval_methods was set to '', we must check if we try dynamic call - // Now we check if we try dynamic call // First we remove white list pattern of using parenthesis then testing if one open parenthesis exists $savescheck = ''; $scheck = $s; @@ -11976,157 +11975,158 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr // Now test if it remains 1 open parenthesis. if (strpos($scheck, '(') !== false) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found call of a function or method without using the direct name of the function): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found call of a function or method without using the direct name of the function): ' . $s, LOG_WARNING); - return ''; - } - } - - // TODO - // We can exclude $ char that are not in dol_eval global, so that are not: - // $db, $langs, $leftmenu, $topmenu, $user, $langs, $objectoffield, $object, $obj, ..., - } - if (is_array($s) || $s === 'Array') { - if ($returnvalue) { - return 'Bad string syntax to evaluate (value is Array): ' . var_export($s, true); - } else { - dol_syslog('Bad string syntax to evaluate (value is Array): ' . var_export($s, true), LOG_WARNING); - return ''; - } - } - - if (!getDolGlobalString('MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL') && strpos($s, '::') !== false) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (double : char is forbidden without setting MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (double : char is forbidden without setting MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL): ' . $s, LOG_WARNING); - return ''; + return 'Bad string syntax to evaluate (mode ' . $onlysimplestring . ', found call of a function or method without using the direct name of the function): ' . $s; } } if (strpos($s, '`') !== false) { - if ($returnvalue) { - return 'Bad string syntax to evaluate (backtick char is forbidden): ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate (backtick char is forbidden): ' . $s, LOG_WARNING); - return ''; + return 'Bad string syntax to evaluate (backtick char is forbidden): ' . $s; + } + + // Disallow also concat operator + if (!getDolGlobalString('MAIN_ALLOW_OBFUSCATION_METHODS_IN_DOL_EVAL')) { + if (preg_match('/[^0-9]+\.[^0-9]+/', $s)) { // We refuse . if not between 2 numbers + return 'Bad string syntax to evaluate (dot char is forbidden if not strictly between 2 numbers): ' . $s; } } - // Disallow also concat - if (!getDolGlobalString('MAIN_ALLOW_OBFUSCATION_METHODS_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 ''; - } + // We exclude string using a $ character that are not an expected global or temporary vars, so that are not: + // $db, $langs, $leftmenu, $topmenu, $user, $langs, $objectoffield, $var.... + $savescheck = ''; + $scheck = $s; + while ($scheck && $savescheck != $scheck) { + $savescheck = $scheck; + $scheck = preg_replace('/\$user->hasRight/', '__VARUSERHASRIGHT__', $scheck); + $scheck = preg_replace('/\(\$db\)/', '__VARDB__', $scheck); + $scheck = preg_replace('/\$langs/', '__VARLANGSTRANS__', $scheck); + $scheck = preg_replace('/\$mysoc/', '__VARMYSOC__', $scheck); + $scheck = preg_replace('/\$action/', '__VARACTION__', $scheck); + $scheck = preg_replace('/\$mainmenu/', '__VARMAINMENU__', $scheck); + $scheck = preg_replace('/\$leftmenu/', '__VARLEFTMENU__', $scheck); + $scheck = preg_replace('/\$websitepage/', '__VARWEBSITEPAGE__', $scheck); + $scheck = preg_replace('/\$website/', '__VARWEBSITE__', $scheck); + $scheck = preg_replace('/\$objectoffield/', '__VAROBJECTOFFIELD__', $scheck); + $scheck = preg_replace('/\$var/', '__VARVAR__', $scheck); + + // Now test if it remains 1 $ + if (strpos($scheck, '$') !== false) { + return 'Bad string syntax to evaluate (found use of $ that does not match one of the following pattern: $user->hasRight, ($db), $langs, $mysoc, $action, $mainmenu, $leftmenu, $website, $websitepage, $objectoffield or $var123): ' . $s; } } // We block use of php exec or php file functions - $forbiddenphpstrings = array('$$', '$_', '}[', ')('); + $forbiddenphpstrings = array('}[', ')('); $forbiddenphpstrings = array_merge($forbiddenphpstrings, array('_ENV', '_SESSION', '_COOKIE', '_GET', '_GLOBAL', '_POST', '_REQUEST', 'ReflectionFunction', 'SplFileObject', 'SplTempFileObject')); // We list all forbidden function as keywords we don't want to see (we don't mind it if is "keyword(" or just "keyword", we don't want "keyword" at all) // We must exclude all functions that allow to execute another function. This includes all function that has a parameter with type "callable" to avoid things // like we can do with array_map and its callable parameter: dol_eval('json_encode(array_map(implode("",["ex","ec"]), ["id"]))', 1, 1, '0') $forbiddenphpfunctions = array(); - $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")); + $forbiddenphpmethods = array(); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("array_all", "array_any", "array_diff_ukey", "array_filter", "array_find", "array_find_key", "array_map", "array_reduce", "array_intersect_uassoc", "array_intersect_ukey", "array_walk", "array_walk_recursive")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("usort", "uasort", "uksort", "preg_replace_callback", "preg_replace_callback_array", "header_register_callback")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("error_log", "set_error_handler", "set_exception_handler", "libxml_set_external_entity_loader", "register_shutdown_function", "register_tick_function", "unregister_tick_function")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("spl_autoload_register", "spl_autoload_unregister", "iterator_apply", "session_set_save_handler")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("forward_static_call", "forward_static_call_array", "register_postsend_function")); + if (empty($dolibarr_main_restrict_eval_methods)) { // If forced to '' + $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")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("ob_start")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("array_all", "array_any", "array_diff_ukey", "array_filter", "array_find", "array_find_key", "array_map", "array_reduce", "array_intersect_uassoc", "array_intersect_ukey", "array_walk", "array_walk_recursive")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("usort", "uasort", "uksort", "preg_replace_callback", "preg_replace_callback_array", "header_register_callback")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("error_log", "set_error_handler", "set_exception_handler", "libxml_set_external_entity_loader", "register_shutdown_function", "register_tick_function", "unregister_tick_function")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("spl_autoload_register", "spl_autoload_unregister", "iterator_apply", "session_set_save_handler")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("forward_static_call", "forward_static_call_array", "register_postsend_function")); - $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", "dol_eval_new", "dol_eval_standard", "executeCLI", "verifCond", "GETPOST", "dolEncrypt", "dolDecrypt")); // native dolibarr functions - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("eval", "create_function", "assert", "mb_ereg_replace")); // function with eval capabilities - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("readline_completion_function", "readline_callback_handler_install")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_compress_dir", "dol_decode", "dol_dir_list", "dol_dir_list_in_database", "dol_delete_file", "dol_delete_dir", "dol_delete_dir_recursive", "dol_copy", "archiveOrBackupFile")); // more dolibarr functions - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("chdir", "dir", "fopen", "file", "file_exists", "file_get_contents", "file_put_contents", "fget", "fgetc", "fgetcsv", "fputs", "fputscsv", "fpassthru", "fscanf", "fseek", "fwrite", "is_file", "is_dir", "is_link", "mkdir", "opendir", "rmdir", "scandir", "symlink", "touch", "unlink", "umask")); - $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("require", "include")); - if (!getDolGlobalString('MAIN_ALLOW_OBFUSCATION_METHODS_IN_DOL_EVAL')) { // We disallow 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_concat", "dol_concatdesc")); // native dolibarr functions - } - // Remove from blacklist the function that are into the whitelist - foreach ($forbiddenphpfunctions as $key => $forbiddenphpfunction) { - if (in_array($forbiddenphpfunction, $dolibarr_main_restrict_eval_methods_array)) { - unset($forbiddenphpfunctions[$key]); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("ob_start")); + + $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", "dol_eval_new", "dol_eval_standard", "executeCLI", "verifCond", "GETPOST", "dolEncrypt", "dolDecrypt")); // native dolibarr functions + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("eval", "create_function", "assert", "mb_ereg_replace")); // function with eval capabilities + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("readline_completion_function", "readline_callback_handler_install")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_compress_dir", "dol_decode", "dol_dir_list", "dol_dir_list_in_database", "dol_delete_file", "dol_delete_dir", "dol_delete_dir_recursive", "dol_copy", "archiveOrBackupFile")); // more dolibarr functions + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("chdir", "dir", "fopen", "file", "file_exists", "file_get_contents", "file_put_contents", "fget", "fgetc", "fgetcsv", "fputs", "fputscsv", "fpassthru", "fscanf", "fseek", "fwrite", "is_file", "is_dir", "is_link", "mkdir", "opendir", "rmdir", "scandir", "symlink", "touch", "unlink", "umask")); + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("require", "include")); + if (!getDolGlobalString('MAIN_ALLOW_OBFUSCATION_METHODS_IN_DOL_EVAL')) { // We disallow 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", "printf", "sprintf")); // name of forbidden functions are split to avoid false positive + $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_concat", "dol_concatdesc")); // native dolibarr functions } - } + // Remove from blacklist the function that are into the whitelist + /*foreach ($forbiddenphpfunctions as $key => $forbiddenphpfunction) { + if (in_array($forbiddenphpfunction, $dolibarr_main_restrict_eval_methods_array)) { + unset($forbiddenphpfunctions[$key]); + } + }*/ - $forbiddenphpmethods = array('invoke', 'invokeArgs'); // Method of ReflectionFunction to execute a function - // Remove from blacklist the function that are into the whitelist - foreach ($forbiddenphpmethods as $key => $forbiddenphpmethod) { - if (in_array($forbiddenphpmethod, $dolibarr_main_restrict_eval_methods_array)) { - unset($forbiddenphpmethods[$key]); - } - } + $forbiddenphpmethods = array_merge($forbiddenphpmethods, array('invoke', 'invokeArgs')); // Methods of ReflectionFunction to execute a function + // Remove from blacklist the function that are into the whitelist + /*foreach ($forbiddenphpmethods as $key => $forbiddenphpmethod) { + if (in_array($forbiddenphpmethod, $dolibarr_main_restrict_eval_methods_array)) { + unset($forbiddenphpmethods[$key]); + } + }*/ - $forbiddenphpregex = 'global\s*\$'; - $forbiddenphpregex .= '|'; - $forbiddenphpregex .= '\b(' . implode('|', $forbiddenphpfunctions) . ')\b'; + $forbiddenphpregex = 'global\s*\$'; + $forbiddenphpregex .= '|'; + $forbiddenphpregex .= '\b(' . implode('|', $forbiddenphpfunctions) . ')\b'; - $forbiddenphpmethodsregex = '->(' . implode('|', $forbiddenphpmethods) . ')'; + $forbiddenphpmethodsregex = '->(' . implode('|', $forbiddenphpmethods) . ')'; - // Now scan all forbidden patterns - do { - $oldstringtoclean = $s; - $s = str_ireplace($forbiddenphpstrings, '__forbiddenstring__', $s); - $s = preg_replace('/' . $forbiddenphpregex . '/i', '__forbiddenstring__', $s); - $s = preg_replace('/' . $forbiddenphpmethodsregex . '/i', '__forbiddenstring__', $s); - //$s = preg_replace('/\$[a-zA-Z0-9_\->\$]+\(/i', '', $s); // Remove $function( call and $mycall->mymethod( - } while ($oldstringtoclean != $s); + // Now scan all forbidden patterns + do { + $oldstringtoclean = $s; + $s = str_ireplace($forbiddenphpstrings, '__forbiddenstring__', $s); + $s = preg_replace('/' . $forbiddenphpregex . '/i', '__forbiddenstring__', $s); + $s = preg_replace('/' . $forbiddenphpmethodsregex . '/i', '__forbiddenstring__', $s); + //$s = preg_replace('/\$[a-zA-Z0-9_\->\$]+\(/i', '', $s); // Remove $function( call and $mycall->mymethod( + } while ($oldstringtoclean != $s); - if (strpos($s, '__forbiddenstring__') !== false) { - dol_syslog('Bad string syntax to evaluate: ' . $s, LOG_WARNING); - if ($returnvalue) { + if (strpos($s, '__forbiddenstring__') !== false) { + dol_syslog('Bad string syntax to evaluate: ' . $s, LOG_WARNING); return 'Bad string syntax to evaluate: ' . $s; - } else { - dol_syslog('Bad string syntax to evaluate: ' . $s); - return ''; } - } + } else { + // Accept only white-listed allowed function and classes + // TODO Get all pattern '/([\s\w]+)\(/', then check that $reg[1] is a defined class or a function into a given list + $pattern = '/([\s\w\'\]\"]+)\(/'; + $matches = array(); + preg_match_all($pattern, $s, $matches); - // Accept only white-listed allowed function and classes - if (!empty($dolibarr_main_restrict_eval_methods)) { - // TODO Get all pattern '/(\w+)\(/', then check that $reg[1] is a defined class or a function into a given list + if (!empty($matches)) { + foreach ($matches[1] as $m) { + $m = trim($m); + if (empty($m)) { + continue; + } + $reg = array(); + if (!preg_match('/new ([A-Z][\w]+)/i', $m, $reg)) { + if (!in_array($m, $dolibarr_main_restrict_eval_methods_array)) { + if ($m != "'" && $m != '"') { + dol_syslog('Bad string syntax to evaluate: ' . $s, LOG_WARNING); + return 'Bad string syntax to evaluate. A function or method "'.$m.'" was called and is not into the parameter $dolibarr_main_restrict_eval_methods of white-listed functions and methods: ' . $s; + } + } + } else { + if ($reg[1] == 'ReflectionFunction') { + dol_syslog('Bad string syntax to evaluate: Class ReflectionFunction is not allowed. ' . $s, LOG_WARNING); + return 'Bad string syntax to evaluate. Class ReflectionFunction is not allowed. ' . $s; + } + } + } + } } //print $s."
\n"; - if ($returnvalue) { - ob_start(); // An evaluation has no reason to output data - $isObBufferActive = true; - $tmps = $hideerrors ? @eval('return ' . $s . ';') : eval('return ' . $s . ';'); - $tmpo = ob_get_clean(); - $isObBufferActive = false; - if ($tmpo) { - print 'Bad string syntax to evaluate. Some data were output when it should not when evaluating: ' . $s; - } - return $tmps; - } else { - dol_syslog('Do not use anymore dol_eval with param returnvalue=0', LOG_WARNING); - if ($hideerrors) { - @eval($s); - } else { - eval($s); - } - return ''; + ob_start(); // An evaluation has no reason to output data + $isObBufferActive = true; + $tmps = $hideerrors ? @eval('return ' . $s . ';') : eval('return ' . $s . ';'); + $tmpo = ob_get_clean(); + $isObBufferActive = false; + if ($tmpo) { + print 'Bad string syntax to evaluate. Some data were output when it should not when evaluating: ' . $s; } + return $tmps; } catch (Error $e) { if ($isObBufferActive) { // Clean up buffer which was left behind due to exception. @@ -12136,11 +12136,7 @@ function dol_eval_standard($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestr $error = 'dol_eval try/catch error : '; $error .= $e->getMessage(); dol_syslog($error, LOG_WARNING); - if ($returnvalue) { - return 'Exception during evaluation: ' . $s; - } else { - return ''; - } + return 'Exception during evaluation: ' . $s; } } diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 51a022efc34..da29c63e615 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -617,6 +617,12 @@ class SecurityTest extends CommonClassTest $conf->global->MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL = 0; $conf->global->MAIN_ALLOW_OBFUSCATION_METHODS_IN_DOL_EVAL = 1; + // We force $dolibarr_main_restrict_eval_methods to empty, so the code will use the old black-list patterns. + global $dolibarr_main_restrict_eval_methods; + print "\ndolibarr_main_restrict_eval_methods = ".$dolibarr_main_restrict_eval_methods."\n"; + + //$dolibarr_main_restrict_eval_methods = array(); + //$resulttest = dol_eval('((getDolGlobalString("MAIN_USE_ADVANCED_PERMS") ? $user->hasRight("user","group_advance","read") : $user->hasRight("user","user","lire")) || $user->admin) && !(isModEnabled("multicompany") && $conf->entity > 1 && getDolGlobalString("MULTICOMPANY_TRANSVERSE_MODE"))', 1, 0); //print "resulttest = ".$resulttest."\n"; @@ -630,22 +636,22 @@ class SecurityTest extends CommonClassTest print "result2 = ".$result."\n"; $this->assertFalse($result); - $s = '((($reloadedobj = new ClassThatDoesNotExists($db)) && ($reloadedobj->fetchNoCompute($objectoffield->fk_product) > 0)) ? \'1\' : \'0\')'; + $s = '((($var1 = new ClassThatDoesNotExists($db)) && ($var1->fetchNoCompute($objectoffield->fk_product) > 0)) ? \'1\' : \'0\')'; $result3a = dol_eval($s, 1, 1, '2'); print "result3a = ".$result3a."\n"; $this->assertStringContainsString('Exception during evaluation: '.$s, $result3a); - $s = '((($reloadedobj = new Project($db)) && ($reloadedobj->fetchNoCompute($objectoffield->fk_product) > 0)) ? \'1\' : \'0\')'; + $s = '((($var1 = new Project($db)) && ($var1->fetchNoCompute($objectoffield->fk_product) > 0)) ? \'1\' : \'0\')'; $result3b = dol_eval($s, 1, 1, '2'); print "result3b = ".$result."\n"; $this->assertEquals('0', $result3b); - $s = '(($reloadedobj = new Task($db)) && ($reloadedobj->fetchNoCompute($object->id) > 0) && ($secondloadedobj = new Project($db)) && ($secondloadedobj->fetchNoCompute($reloadedobj->fk_project) > 0)) ? $secondloadedobj->ref : "Parent project not found"'; + $s = '(($var1 = new Task($db)) && ($var1->fetchNoCompute($objectoffield->id) > 0) && ($var2 = new Project($db)) && ($var2->fetchNoCompute($var1->fk_project) > 0)) ? $var2->ref : "Parent project not found"'; $result = (string) dol_eval($s, 1, 1, '2'); print "result3c = ".$result."\n"; $this->assertEquals('Parent project not found', $result); - $s = '(($reloadedobj = new Task($db)) && ($reloadedobj->fetchNoCompute($object->id) > 0) && ($secondloadedobj = new Project($db)) && ($secondloadedobj->fetchNoCompute($reloadedobj->fk_project) > 0)) ? $secondloadedobj->ref : \'Parent project not found\''; + $s = '(($var1 = new Task($db)) && ($var1->fetchNoCompute($objectoffield->id) > 0) && ($var2 = new Project($db)) && ($var2->fetchNoCompute($var1->fk_project) > 0)) ? $var2->ref : \'Parent project not found\''; $result = (string) dol_eval($s, 1, 1, '2'); print "result4 = ".$result."\n"; $this->assertEquals('Parent project not found', $result, 'Test 4'); @@ -659,12 +665,12 @@ class SecurityTest extends CommonClassTest print "result6 = ".$result."\n"; $this->assertEquals('1', $result, 'Test 5'); + /* $s = 'MyClass::MyMethod()'; $result = dol_eval($s, 1, 1, '2'); print "result7 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate (double : char is forbidden without setting MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL)', $result); - - + $this->assertStringContainsString('Bad string syntax to evaluate (double : char is forbidden without setting MAIN_ALLOW_DOUBLE_COLON_IN_DOL_EVAL)', $result, 'The string was not detected as evil'); + */ /* not allowed. Not a one line eval string $result = (string) dol_eval('if ($a == 1) { }', 1, 1); @@ -718,21 +724,21 @@ class SecurityTest extends CommonClassTest print "result7 = ".$result."\n"; $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); + $result = (string) dol_eval('$var1=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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); - $result = (string) dol_eval('$a=exec(\'ls\')', 1, 1); + $result = (string) dol_eval('$var1=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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); - $result = (string) dol_eval('$a=exec ("ls")', 1, 1); + $result = (string) dol_eval('$var1=exec ("ls")', 1, 1); print "result8 = ".$result."\n"; $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $result = (string) dol_eval("strrev('metsys') ('whoami')", 1, 1); print "result8b = ".$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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $conf->global->MAIN_ALLOW_OBFUSCATION_METHODS_IN_DOL_EVAL = 0; @@ -746,11 +752,11 @@ class SecurityTest extends CommonClassTest $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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $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"; @@ -825,7 +831,7 @@ class SecurityTest extends CommonClassTest $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'); + $this->assertStringContainsString('Bad string syntax to evaluate (dot char is forbidden if not strictly between 2 numbers)', $result, 'Test concat - The string was not reported as a bad syntax when it should'); // Not allowed @@ -847,11 +853,15 @@ 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'); + $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 it 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'); + + $result = (string) dol_eval('$$a', 1, 0); + print "result25 = ".$result."\n"; + $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result), 'Test 25 - The string was not detected as evil - Can\'t find the string Bad string syntax when i should'); }