Merge branch '15.0' of git@github.com:Dolibarr/dolibarr.git into develop

Conflicts:
	htdocs/core/lib/functions.lib.php

htdocs/bookmarks/card.php

@@ -160,7 +160,7 @@ if ($action == 'create') {
 
 	print '<table class="border centpercent tableforfieldcreate">';
 
-	print '<tr><td class="titlefieldcreate fieldrequired">'.$langs->trans("BookmarkTitle").'</td><td><input id="titlebookmark" class="flat minwidth300" name="title" value="'.dol_escape_htmltag($title).'"></td><td class="hideonsmartphone"><span class="opacitymedium">'.$langs->trans("SetHereATitleForLink").'</span></td></tr>';
+	print '<tr><td class="titlefieldcreate fieldrequired">'.$langs->trans("BookmarkTitle").'</td><td><input id="titlebookmark" class="flat minwidth250" name="title" value="'.dol_escape_htmltag($title).'"></td><td class="hideonsmartphone"><span class="opacitymedium">'.$langs->trans("SetHereATitleForLink").'</span></td></tr>';
 	dol_set_focus('#titlebookmark');
 
 	// Url
@@ -230,9 +230,9 @@ if ($id > 0 && !preg_match('/^add/i', $action)) {
 
 	print '</td><td>';
 	if ($action == 'edit') {
-		print '<input class="flat minwidth300" name="title" value="'.(GETPOSTISSET("title") ? GETPOST("title", '', 2) : $object->title).'">';
+		print '<input class="flat minwidth250" name="title" value="'.(GETPOSTISSET("title") ? GETPOST("title", '', 2) : $object->title).'">';
 	} else {
-		print $object->title;
+		print dol_escape_htmltag($object->title);
 	}
 	print '</td></tr>';
 

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."<br>\n";

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

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);