Skip to content

Commit

Permalink
Complete phpunit and tests to avoid use of non expected function
Browse files Browse the repository at this point in the history
  • Loading branch information
eldy committed Dec 18, 2024
1 parent 27ac538 commit 7f4b2b0
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 69 deletions.
50 changes: 26 additions & 24 deletions htdocs/comm/card.php
Original file line number Diff line number Diff line change
Expand Up @@ -1712,35 +1712,37 @@
}

// Add invoice
if ($user->socid == 0) {
if (isModEnabled('deplacement') && $object->status == 1) {
$langs->load("trips");
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/compta/deplacement/card.php?socid='.$object->id.'&amp;action=create">'.$langs->trans("AddTrip").'</a></div>';
}
if (isModEnabled('deplacement') && $object->status == 1) {
$langs->load("trips");
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/compta/deplacement/card.php?socid='.$object->id.'&amp;action=create">'.$langs->trans("AddTrip").'</a></div>';
}

if (isModEnabled('invoice') && $object->status == 1) {
if (!$user->hasRight('facture', 'creer')) {
$langs->load("bills");
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("NotAllowed")).'" href="#">'.$langs->trans("AddBill").'</a></div>';
} else {
$langs->loadLangs(array("orders", "bills"));

if (isModEnabled('invoice') && $object->status == 1) {
if (!$user->hasRight('facture', 'creer')) {
$langs->load("bills");
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("NotAllowed")).'" href="#">'.$langs->trans("AddBill").'</a></div>';
if ($object->client != 0 && $object->client != 2) {
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/compta/facture/card.php?action=create&socid='.$object->id.'">'.$langs->trans("AddBill").'</a></div>';
} else {
$langs->loadLangs(array("orders", "bills"));

if (isModEnabled('order')) {
if ($object->client != 0 && $object->client != 2) {
if (!empty($orders2invoice) && $orders2invoice > 0) {
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/commande/list.php?socid='.$object->id.'&search_billed=0&autoselectall=1">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
} else {
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("NoOrdersToInvoice")).'" href="#">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
}
} else {
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("ThirdPartyMustBeEditAsCustomer")).'" href="#">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
}
}
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("ThirdPartyMustBeEditAsCustomer")).'" href="#">'.$langs->trans("AddBill").'</a></div>';
}
}
}

if (isModEnabled('invoice') && $object->status == 1) {
if ($user->hasRight('facture', 'creer')) {
if (isModEnabled('order')) {
if ($object->client != 0 && $object->client != 2) {
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/compta/facture/card.php?action=create&socid='.$object->id.'">'.$langs->trans("AddBill").'</a></div>';
if (!empty($orders2invoice) && $orders2invoice > 0) {
print '<div class="inline-block divButAction"><a class="butAction" href="'.DOL_URL_ROOT.'/commande/list.php?socid='.$object->id.'&search_billed=0&autoselectall=1">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
} else {
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("NoOrdersToInvoice")).'" href="#">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
}
} else {
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("ThirdPartyMustBeEditAsCustomer")).'" href="#">'.$langs->trans("AddBill").'</a></div>';
print '<div class="inline-block divButAction"><a class="butActionRefused classfortooltip" title="'.dol_escape_js($langs->trans("ThirdPartyMustBeEditAsCustomer")).'" href="#">'.$langs->trans("CreateInvoiceForThisCustomer").'</a></div>';
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions htdocs/core/lib/functions.lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -10586,6 +10586,7 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1'
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('/^!?[a-zA-Z0-9_]+\(/', '__FUNCTION__', $scheck); // accept parenthesis in 'function(' and '!function('
Expand Down Expand Up @@ -10646,22 +10647,26 @@ function dol_eval($s, $returnvalue = 1, $hideerrors = 1, $onlysimplestring = '1'
$forbiddenphpstrings = array('$$', '$_', '}[');
$forbiddenphpstrings = array_merge($forbiddenphpstrings, array('_ENV', '_SESSION', '_COOKIE', '_GET', '_POST', '_REQUEST', 'ReflectionFunction'));

// 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("fopen", "file_put_contents", "fputs", "fputscsv", "fwrite", "fpassthru", "require", "include", "mkdir", "rmdir", "symlink", "touch", "unlink", "umask"));
$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"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("function", "call_user_func", "call_user_func_array"));
$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", "executeCLI", "verifCond")); // native dolibarr functions
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("eval", "create_function", "assert", "mb_ereg_replace")); // function with eval capabilities
$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"));

$forbiddenphpmethods = array('invoke', 'invokeArgs'); // Method of ReflectionFunction to execute a function

$forbiddenphpregex = 'global\s+\$|\b('.implode('|', $forbiddenphpfunctions).')\b';
$forbiddenphpregex = 'global\s*\$';
$forbiddenphpregex .= '|';
$forbiddenphpregex .= '\b('.implode('|', $forbiddenphpfunctions).')\b';

$forbiddenphpmethodsregex = '->('.implode('|', $forbiddenphpmethods).')';

Expand Down
25 changes: 19 additions & 6 deletions htdocs/core/lib/website2.lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -722,22 +722,26 @@ function checkPHPCode(&$phpfullcodestringold, &$phpfullcodestring)

// Then check forbidden commands
if (!$error) {
$forbiddenphpstrings = array('$$', '}[');
$forbiddenphpstrings = array_merge($forbiddenphpstrings, array('ReflectionFunction'));
$forbiddenphpstrings = array('$$', '$_', '}[');
//$forbiddenphpstrings = array_merge($forbiddenphpstrings, array('_ENV', '_SESSION', '_COOKIE', '_GET', '_POST', '_REQUEST', 'ReflectionFunction'));
$forbiddenphpstrings = array_merge($forbiddenphpstrings, array('_ENV', 'ReflectionFunction'));

$forbiddenphpfunctions = array();
//$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("call_user_func"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("call_user_func", "call_user_func_array"));
//$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("require", "include", "require_once", "include_once"));
if (!getDolGlobalString('WEBSITE_PHP_ALLOW_EXEC')) { // If option is not on, we disallow functions to execute commands
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("exec", "passthru", "shell_exec", "system", "proc_open", "popen"));
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("dol_eval", "executeCLI", "verifCond")); // native dolibarr functions
$forbiddenphpfunctions = array_merge($forbiddenphpfunctions, array("eval", "create_function", "assert", "mb_ereg_replace")); // function with eval capabilities
}
if (!getDolGlobalString('WEBSITE_PHP_ALLOW_WRITE')) { // If option is not on, we disallow functions to write files
$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"));

$forbiddenphpmethods = array('invoke', 'invokeArgs'); // Method of ReflectionFunction to execute a function

Expand All @@ -748,13 +752,22 @@ function checkPHPCode(&$phpfullcodestringold, &$phpfullcodestring)
break;
}
}
foreach ($forbiddenphpfunctions as $forbiddenphpfunction) {
if (preg_match('/'.$forbiddenphpfunction.'\s*\(/ims', $phpfullcodestring)) {
/* replaced with next block
foreach ($forbiddenphpfunctions as $forbiddenphpfunction) { // Check "function(" but also "'function'(" and "function ("
if (preg_match('/'.$forbiddenphpfunction.'[\'\s]*\(/ims', $phpfullcodestring)) {
$error++;
setEventMessages($langs->trans("DynamicPHPCodeContainsAForbiddenInstruction", $forbiddenphpfunction), null, 'errors');
break;
}
}*/
foreach ($forbiddenphpfunctions as $forbiddenphpfunction) { // Check "function" whatever is "function(" or "function'(" or "function (" or "function"
if (preg_match('/\b'.$forbiddenphpfunction.'\b/ims', $phpfullcodestring)) {
$error++;
setEventMessages($langs->trans("DynamicPHPCodeContainsAForbiddenInstruction", $forbiddenphpfunction), null, 'errors');
break;
}
}

foreach ($forbiddenphpmethods as $forbiddenphpmethod) {
if (preg_match('/->'.$forbiddenphpmethod.'/ims', $phpfullcodestring)) {
$error++;
Expand Down Expand Up @@ -803,7 +816,7 @@ function checkPHPCode(&$phpfullcodestringold, &$phpfullcodestring)
}
}

// No need to block $conf->global->aaa() because PHP try to run method aaa an not function into $conf->global->aaa.
// No need to block $conf->global->aaa() because PHP try to run the method aaa of $conf->global and not the function into $conf->global->aaa.

// Then check if installmodules does not block dynamic PHP code change.
if ($phpfullcodestringold != $phpfullcodestring) {
Expand Down
2 changes: 2 additions & 0 deletions test/phpunit/AllTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ public static function suite()
$suite->addTestSuite('SecurityTest');
require_once dirname(__FILE__).'/SecurityGETPOSTTest.php';
$suite->addTestSuite('SecurityGETPOSTTest');
require_once dirname(__FILE__).'/SecurityLoginTest.php';
$suite->addTestSuite('SecurityLoginTest');

require_once dirname(__FILE__).'/UserTest.php';
$suite->addTestSuite('UserTest');
Expand Down
106 changes: 106 additions & 0 deletions test/phpunit/SecurityLoginTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
<?php
/* Copyright (C) 2010 Laurent Destailleur <eldy@users.sourceforge.net>
* Copyright (C) 2023 Alexandre Janniaux <alexandre.janniaux@gmail.com>
* Copyright (C) 2024 Frédéric France <frederic.france@free.fr>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
* or see https://www.gnu.org/
*/

/**
* \file test/phpunit/SecurityTest.php
* \ingroup test
* \brief PHPUnit test
* \remarks To run this script as CLI: phpunit filename.php
*/

global $conf,$user,$langs,$db;
//define('TEST_DB_FORCE_TYPE','mysql'); // This is to force using mysql driver
//require_once 'PHPUnit/Autoload.php';

if (! defined('NOREQUIRESOC')) {
define('NOREQUIRESOC', '1');
}
if (! defined('NOCSRFCHECK')) {
define('NOCSRFCHECK', '1');
}
if (! defined('NOTOKENRENEWAL')) {
define('NOTOKENRENEWAL', '1');
}
if (! defined('NOREQUIREMENU')) {
define('NOREQUIREMENU', '1'); // If there is no menu to show
}
if (! defined('NOREQUIREHTML')) {
define('NOREQUIREHTML', '1'); // If we don't need to load the html.form.class.php
}
if (! defined('NOREQUIREAJAX')) {
define('NOREQUIREAJAX', '1');
}
if (! defined("NOLOGIN")) {
define("NOLOGIN", '1'); // If this page is public (can be called outside logged session)
}
if (! defined("NOSESSION")) {
define("NOSESSION", '1');
}

require_once dirname(__FILE__).'/../../htdocs/main.inc.php'; // We force include of main.inc.php instead of master.inc.php even if we are in CLI mode because it contains a lot of security components we want to test.
require_once dirname(__FILE__).'/../../htdocs/core/lib/security.lib.php';
require_once dirname(__FILE__).'/../../htdocs/core/lib/security2.lib.php';
require_once dirname(__FILE__).'/CommonClassTest.class.php';

if (empty($user->id)) {
print "Load permissions for admin user nb 1\n";
$user->fetch(1);
$user->loadRights();
}
$conf->global->MAIN_DISABLE_ALL_MAILS = 1;


/**
* Class for PHPUnit tests
*
* @backupGlobals disabled
* @backupStaticAttributes enabled
* @remarks backupGlobals must be disabled to have db,conf,user and lang not erased.
*/
class SecurityLoginTest extends CommonClassTest
{
/**
* testCheckLoginPassEntity
*
* @return void
*/
public function testCheckLoginPassEntity()
{
$login = checkLoginPassEntity('loginbidon', 'passwordbidon', 1, array('dolibarr'));
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, '');

$login = checkLoginPassEntity('admin', 'passwordbidon', 1, array('dolibarr'));
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, '');

$login = checkLoginPassEntity('admin', 'admin', 1, array('dolibarr')); // Should works because admin/admin exists
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, 'admin', 'The test to check if pass of user "admin" is "admin" has failed');

$login = checkLoginPassEntity('admin', 'admin', 1, array('http','dolibarr')); // Should work because of second authentication method
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, 'admin');

$login = checkLoginPassEntity('admin', 'admin', 1, array('forceuser'));
print __METHOD__." login=".$login."\n";
$this->assertEquals('', $login, 'Error'); // Expected '' because should failed because login 'auto' does not exists
}
}
46 changes: 11 additions & 35 deletions test/phpunit/SecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,13 +636,13 @@ public function testDolEval()
$this->assertEquals('Bad string syntax to evaluate: new __forbiddenstring__(\'abc\')', $result);


$result = (string) dol_eval('$a=function() { }; $a;', 1, 1, '0');
print "result5 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result);
$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));

$result = (string) dol_eval('$a=function() { }; $a;', 1, 1, '1');
print "result6 = ".$result."\n";
$this->assertStringContainsString('Bad string syntax to evaluate', $result);
$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));

$result = (string) dol_eval('$a=exec("ls");', 1, 1);
print "result7 = ".$result."\n";
Expand Down Expand Up @@ -723,6 +723,11 @@ public function testDolEval()
$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');


$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');
}

/**
Expand Down Expand Up @@ -966,33 +971,4 @@ public function testDolHtmlWithNoJs()

return 0;
}


/**
* testCheckLoginPassEntity
*
* @return void
*/
public function testCheckLoginPassEntity()
{
$login = checkLoginPassEntity('loginbidon', 'passwordbidon', 1, array('dolibarr'));
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, '');

$login = checkLoginPassEntity('admin', 'passwordbidon', 1, array('dolibarr'));
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, '');

$login = checkLoginPassEntity('admin', 'admin', 1, array('dolibarr')); // Should works because admin/admin exists
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, 'admin', 'The test to check if pass of user "admin" is "admin" has failed');

$login = checkLoginPassEntity('admin', 'admin', 1, array('http','dolibarr')); // Should work because of second authentication method
print __METHOD__." login=".$login."\n";
$this->assertEquals($login, 'admin');

$login = checkLoginPassEntity('admin', 'admin', 1, array('forceuser'));
print __METHOD__." login=".$login."\n";
$this->assertEquals('', $login, 'Error'); // Expected '' because should failed because login 'auto' does not exists
}
}
Loading

0 comments on commit 7f4b2b0

Please sign in to comment.