diff --git a/htdocs/bookmarks/card.php b/htdocs/bookmarks/card.php index 8833978e054..bbc92a83ef1 100644 --- a/htdocs/bookmarks/card.php +++ b/htdocs/bookmarks/card.php @@ -160,7 +160,7 @@ if ($action == 'create') { print ''; - print ''; + print ''; dol_set_focus('#titlebookmark'); // Url @@ -230,9 +230,9 @@ if ($id > 0 && !preg_match('/^add/i', $action)) { print ''; diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 084eb277b27..bc07728080f 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -8297,7 +8297,7 @@ function dol_getIdFromCode($db, $key, $tablename, $fieldkey = 'code', $fieldid = * Verify if condition in string is ok or not * * @param string $strToEvaluate String with condition to check - * @return boolean True or False. Note: It returns also True if $strToEvaluate is '' + * @return boolean True or False. Note: It returns also True if $strToEvaluate is ''. False if error */ function verifCond($strToEvaluate) { @@ -8310,8 +8310,10 @@ function verifCond($strToEvaluate) if (isset($strToEvaluate) && $strToEvaluate !== '') { //$str = 'if(!('.$strToEvaluate.')) $rights = false;'; //dol_eval($str, 0, 1, '2'); // The dol_eval must contains all the global $xxx used into a condition + //var_dump($strToEvaluate); $rep = dol_eval($strToEvaluate, 1, 1, '1'); // The dol_eval must contains all the global $xxx used into a condition - $rights = ($rep ? true : false); + $rights = (($rep && strpos($rep, 'Bad string syntax to evaluate') === false) ? true : false); + //var_dump($rights); } return $rights; } @@ -8323,7 +8325,7 @@ function verifCond($strToEvaluate) * @param string $s String to evaluate * @param int $returnvalue 0=No return (used to execute eval($a=something)). 1=Value of eval is returned (used to eval($something)). * @param int $hideerrors 1=Hide errors - * @param string $onlysimplestring 0=Accept all chars, 1=Accept only simple string with char 'a-z0-9\s$_->&|=';, 2=Accept also '!?():";' + * @param string $onlysimplestring 0=Accept all chars, 1=Accept only simple string with char 'a-z0-9\s^$_->&|=!?():"\',/' and restrict use of (, 2=Accept also ';' and no restriction on (. * @return mixed Nothing or return result of eval */ function dol_eval($s, $returnvalue = 0, $hideerrors = 1, $onlysimplestring = '1') @@ -8338,22 +8340,23 @@ function dol_eval($s, $returnvalue = 0, $hideerrors = 1, $onlysimplestring = '1' global $obj; // To get $obj used into list when dol_eval is used for computed fields and $obj is not yet $object global $soc; // For backward compatibility - // Test on dangerous char (used for RCE), we allow only characters to make PHP variable testing. + // Test on dangerous char (used for RCE), we allow only characters to make PHP variable testing if ($onlysimplestring == '1') { - //print preg_quote('$_->&|', '/'); - if (preg_match('/[^a-z0-9\s'.preg_quote('$_->&|=!?():"', '/').']/i', $s)) { + // We must accept: '1 && getDolGlobalInt("doesnotexist1") && $conf->global->MAIN_FEATURES_LEVEL' + // We must accept: '$conf->barcode->enabled && preg_match(\'/^(AAA|BBB)/\',$leftmenu)' + if (preg_match('/[^a-z0-9\s'.preg_quote('^$_->&|=!?():"\',/', '/').']/i', $s)) { if ($returnvalue) { return 'Bad string syntax to evaluate (found chars that are not chars for simplestring): '.$s; } else { dol_syslog('Bad string syntax to evaluate (found chars that are not chars for simplestring): '.$s); return ''; } - // TODO We can exclude all () that is not ...($db) and getDolGlobalInt( and getDolGlobalString( + // TODO We can exclude all () that is not '($db)' and 'getDolGlobalInt(' and 'getDolGlobalString(' and 'preg_match(' // ... } } elseif ($onlysimplestring == '2') { - //print preg_quote('$_->&|', '/'); - if (preg_match('/[^a-z0-9\s'.preg_quote('$_->&|=!?():";', '/').']/i', $s)) { + // We must accept: (($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" + if (preg_match('/[^a-z0-9\s'.preg_quote('^$_->&|=!?():"\',/;[]', '/').']/i', $s)) { if ($returnvalue) { return 'Bad string syntax to evaluate (found chars that are not chars for simplestring): '.$s; } else { @@ -8363,20 +8366,35 @@ function dol_eval($s, $returnvalue = 0, $hideerrors = 1, $onlysimplestring = '1' } } if (strpos($s, '::') !== false) { - return 'Bad string syntax to evaluate (double : char is forbidden): '.$s; + if ($returnvalue) { + return 'Bad string syntax to evaluate (double : char is forbidden): '.$s; + } else { + dol_syslog('Bad string syntax to evaluate (double : char is forbidden): '.$s); + return ''; + } } if (strpos($s, '`') !== false) { - return 'Bad string syntax to evaluate (backtick char is forbidden): '.$s; + 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); + return ''; + } } if (strpos($s, '.') !== false) { - return 'Bad string syntax to evaluate (dot char is forbidden): '.$s; + 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); + return ''; + } } // We block use of php exec or php file functions $forbiddenphpstrings = array('$$'); $forbiddenphpstrings = array_merge($forbiddenphpstrings, array('_ENV', '_SESSION', '_COOKIE', '_GET', '_POST', '_REQUEST')); - $forbiddenphpfunctions = array("exec", "passthru", "shell_exec", "system", "proc_open", "popen", "eval", "dol_eval", "executeCLI"); + $forbiddenphpfunctions = array("exec", "passthru", "shell_exec", "system", "proc_open", "popen", "eval", "dol_eval", "executeCLI", 'verifCond'); $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("fopen", "file_put_contents", "fputs", "fputscsv", "fwrite", "fpassthru", "require", "include", "mkdir", "rmdir", "symlink", "touch", "unlink", "umask")); $forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("function", "call_user_func")); @@ -8391,7 +8409,12 @@ function dol_eval($s, $returnvalue = 0, $hideerrors = 1, $onlysimplestring = '1' if (strpos($s, '__forbiddenstring__') !== false) { dol_syslog('Bad string syntax to evaluate: '.$s, LOG_WARNING); - return 'Bad string syntax to evaluate: '.$s; + if ($returnvalue) { + return 'Bad string syntax to evaluate: '.$s; + } else { + dol_syslog('Bad string syntax to evaluate: '.$s); + return ''; + } } //print $s."
\n"; diff --git a/htdocs/core/modules/modBarcode.class.php b/htdocs/core/modules/modBarcode.class.php index 877dca880f7..ced58f9b6c4 100644 --- a/htdocs/core/modules/modBarcode.class.php +++ b/htdocs/core/modules/modBarcode.class.php @@ -33,7 +33,6 @@ include_once DOL_DOCUMENT_ROOT.'/core/modules/DolibarrModules.class.php'; */ class modBarcode extends DolibarrModules { - /** * Constructor. Define names, constants, directories, boxes, permissions * @@ -94,6 +93,7 @@ class modBarcode extends DolibarrModules // Main menu entries $r = 0; + // A menu entry for the Tools top menu $this->menu[$r] = array( 'fk_menu'=>'fk_mainmenu=tools', // Use 'fk_mainmenu=xxx' or 'fk_mainmenu=xxx,fk_leftmenu=yyy' where xxx is mainmenucode and yyy is a leftmenucode 'mainmenu'=>'tools', @@ -111,6 +111,7 @@ class modBarcode extends DolibarrModules ); $r++; + // A menu entry for the left menu $this->menu[$r] = array( 'fk_menu'=>'fk_mainmenu=home,fk_leftmenu=admintools', // Use 'fk_mainmenu=xxx' or 'fk_mainmenu=xxx,fk_leftmenu=yyy' where xxx is mainmenucode and yyy is a leftmenucode 'type'=>'left', // This is a Left menu entry diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 8be8c07f887..f64a0202aa6 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -877,12 +877,12 @@ class SecurityTest extends PHPUnit\Framework\TestCase include_once DOL_DOCUMENT_ROOT.'/projet/class/task.class.php'; $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"'; - $result=dol_eval($s, 1, 1, ''); + $result=dol_eval($s, 1, 1, '2'); print "result = ".$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\''; - $result=dol_eval($s, 1, 1, ''); + $result=dol_eval($s, 1, 1, '2'); print "result = ".$result."\n"; $this->assertEquals('Parent project not found', $result); @@ -914,7 +914,24 @@ class SecurityTest extends PHPUnit\Framework\TestCase print "result = ".$result."\n"; $this->assertContains('Bad string syntax to evaluate', $result); + global $leftmenu; // Used into strings to eval + + $leftmenu = 'AAA'; + $conf->barcode->enabled = 1; + $result=dol_eval('$conf->barcode->enabled && preg_match(\'/^(AAA|BBB)/\',$leftmenu)', 1, 1, '1'); + print "result = ".$result."\n"; + $this->assertTrue($result); + + // Same with syntax error + $leftmenu = 'XXX'; + $conf->barcode->enabled = 1; + $result=dol_eval('$conf->barcode->enabled && preg_match(\'/^(AAA|BBB)/\',$leftmenu)', 1, 1, '1'); + print "result = ".$result."\n"; + $this->assertFalse($result); + + // Case with param onlysimplestring = 1 + $result=dol_eval('1 && getDolGlobalInt("doesnotexist1") && $conf->global->MAIN_FEATURES_LEVEL', 1, 0); // Should return false and not a 'Bad string syntax to evaluate ...' print "result = ".$result."\n"; $this->assertFalse($result);
'.$langs->trans("BookmarkTitle").''.$langs->trans("SetHereATitleForLink").'
'.$langs->trans("BookmarkTitle").''.$langs->trans("SetHereATitleForLink").'
'; if ($action == 'edit') { - print 'title).'">'; + print 'title).'">'; } else { - print $object->title; + print dol_escape_htmltag($object->title); } print '