From fcc344f9da6b4d99c408833cd02f0b2d3ae87db4 Mon Sep 17 00:00:00 2001 From: "Laurent Destailleur (aka Eldy)" Date: Mon, 23 Dec 2024 11:57:19 +0100 Subject: [PATCH] Security - More robust dol_eval function after vulnerability report by Muhammad Zeeshan (Xib3rR4dAr) --- htdocs/core/lib/functions.lib.php | 13 +++++--- test/phpunit/SecurityTest.php | 51 ++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index ef0e68c369e..fcf8c8407bf 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -10564,9 +10564,9 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1' } 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 clean eval string): '.$s; + 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 clean eval string): '.$s, LOG_WARNING); + 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 ''; } } @@ -10581,15 +10581,17 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1' } } - // Now we check if we try dynamic call (by removing white list pattern of using parenthesis then testing if a parenthesis exists) + // 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; while ($scheck && $savescheck != $scheck) { $savescheck = $scheck; $scheck = preg_replace('/->[a-zA-Z0-9_]+\(/', '->__METHOD__', $scheck); // accept parenthesis in '...->method(...' $scheck = preg_replace('/::[a-zA-Z0-9_]+\(/', '->__METHOD__', $scheck); // accept parenthesis in '...::method(...' - $scheck = preg_replace('/^\(/', '__PARENTHESIS__ ', $scheck); // accept parenthesis in '(...'. Must replace with __PARENTHESIS__ with a space after to allow following substitutions - $scheck = preg_replace('/\s\(/', '__PARENTHESIS__ ', $scheck); // accept parenthesis in '... (' like in 'if ($a == 1)'. Must replace with __PARENTHESIS__ with a space after to allow following substitutions + $scheck = preg_replace('/^\(+/', '__PARENTHESIS__ ', $scheck); // accept parenthesis in '(...'. Must replace with "__PARENTHESIS__ with a space after "to allow following substitutions + $scheck = preg_replace('/\&\&\s+\(/', '__ANDPARENTHESIS__ ', $scheck); // accept parenthesis in '... (' like in '&& (...'. Must replace with "__PARENTHESIS__ with a space after" to allow following substitutions + $scheck = preg_replace('/\|\|\s+\(/', '__ORPARENTHESIS__ ', $scheck); // accept parenthesis in '... (' like in '|| (...'. Must replace with "__PARENTHESIS__ with a space after" to allow following substitutions $scheck = preg_replace('/^!?[a-zA-Z0-9_]+\(/', '__FUNCTION__', $scheck); // accept parenthesis in 'function(' and '!function(' $scheck = preg_replace('/\s!?[a-zA-Z0-9_]+\(/', '__FUNCTION__', $scheck); // accept parenthesis in '... function(' and '... !function(' $scheck = preg_replace('/^!\(/', '__NOTANDPARENTHESIS__', $scheck); // accept parenthesis in '!(' @@ -10598,6 +10600,7 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1' } //print 'scheck='.$scheck." : ".strpos($scheck, '(')."
\n"; + // Now test if it remains 1 one 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; diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 9b5db827bc3..45f962a3b47 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -625,52 +625,66 @@ class SecurityTest extends CommonClassTest print "result4 = ".$result."\n"; $this->assertEquals('Parent project not found', $result); + /* not allowed. Not a one line eval string + $result = (string) dol_eval('if ($a == 1) { }', 1, 1); + print "result4b = ".$result."\n"; + $this->assertEquals('aaa', $result); + */ + + // Now string not allowed + $s = 'new abc->invoke(\'whoami\')'; $result = (string) dol_eval($s, 1, 1, '2'); print "result = ".$result."\n"; - $this->assertEquals('Bad string syntax to evaluate: new abc__forbiddenstring__(\'whoami\')', $result); + $this->assertEquals('Bad string syntax to evaluate: new abc__forbiddenstring__(\'whoami\')', $result, 'The string was not detected as evil'); $s = 'new ReflectionFunction(\'abc\')'; $result = (string) dol_eval($s, 1, 1, '2'); print "result = ".$result."\n"; - $this->assertEquals('Bad string syntax to evaluate: new __forbiddenstring__(\'abc\')', $result); + $this->assertEquals('Bad string syntax to evaluate: new __forbiddenstring__(\'abc\')', $result, 'The string was not detected as evil'); $result = dol_eval('$a=function() { }; $a', 1, 1, '0'); // result of dol_eval may be an object Closure print "result5 = ".json_encode($result)."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result)); + $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result), 'The string was not detected as evil'); $result = dol_eval('$a=function() { }; $a();', 1, 1, '1'); print "result6 = ".json_encode($result)."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result)); + $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); print "result7 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $result = (string) dol_eval('$a=exec ("ls")', 1, 1); print "result8 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result); + $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', $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); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $result = (string) dol_eval('`ls`', 1, 0); print "result10 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $result = (string) dol_eval("('ex'.'ec')('echo abc')", 1, 0); print "result11 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result); + $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"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'The string was not detected as evil'); $result = dol_eval("90402.38+267678+0", 1, 1, 1); print "result13 = ".$result."\n"; - $this->assertEquals('358080.38', $result); + $this->assertEquals('358080.38', $result, 'The string was not detected as evil'); + + // Must be allowed global $leftmenu; // Used into strings to eval @@ -706,28 +720,29 @@ class SecurityTest extends CommonClassTest print "result18 = ".$result."\n"; $this->assertFalse($result); + // Not allowed + $a = 'ab'; $result = (string) dol_eval("(\$a.'s')", 1, 0); print "result19 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 19'); + $this->assertStringContainsString('Bad string syntax to evaluate', $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'); + $this->assertStringContainsString('Bad string syntax to evaluate', $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"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 21'); + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 21 - The string was not detected as evil'); $result = (string) dol_eval('($a = "ex") && ($b = "ec") && ($cmd = "$a$b") && $cmd ("curl localhost:5555")', 1, 0); print "result22 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 22'); - + $this->assertStringContainsString('Bad string syntax to evaluate', $result, 'Test 22 - The string was not detected as evil'); $result = (string) dol_eval('\'exec\'("aaa")', 1, 0); - print "result1 = ".$result."\n"; - $this->assertStringContainsString('Bad string syntax to evaluate', json_encode($result), 'Cant find the string Bad string syntaxwhen i should'); + 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'); } /**