diff --git a/htdocs/core/lib/website2.lib.php b/htdocs/core/lib/website2.lib.php index fb4cfa1b520..768f735fe39 100644 --- a/htdocs/core/lib/website2.lib.php +++ b/htdocs/core/lib/website2.lib.php @@ -718,7 +718,12 @@ function checkPHPCode($phpfullcodestringold, $phpfullcodestring) break; } } - // Check dynamic functions $xxx( + // Deny dynamic functions '${a}(' or '$a[b](' - So we refuse '}(' and '](' + if (preg_match('/[}\]]\(/ims', $phpfullcodestring)) { + $error++; + setEventMessages($langs->trans("DynamicPHPCodeContainsAForbiddenInstruction", ']('), null, 'errors'); + } + // Deny dynamic functions $xxx( if (preg_match('/\$[a-z0-9_]+\(/ims', $phpfullcodestring)) { $error++; setEventMessages($langs->trans("DynamicPHPCodeContainsAForbiddenInstruction", '$...('), null, 'errors'); @@ -732,6 +737,7 @@ function checkPHPCode($phpfullcodestringold, $phpfullcodestring) if (!$error) { $dolibarrdataroot = preg_replace('/([\\/]+)$/i', '', DOL_DATA_ROOT); $allowimportsite = true; + include_once DOL_DOCUMENT_ROOT.'/core/lib/files.lib.php'; if (dol_is_file($dolibarrdataroot.'/installmodules.lock')) { $allowimportsite = false; } diff --git a/test/phpunit/Website.class.php b/test/phpunit/Website.class.php index 50d0c16453d..364235bcc41 100644 --- a/test/phpunit/Website.class.php +++ b/test/phpunit/Website.class.php @@ -54,12 +54,17 @@ if (! defined("NOSESSION")) { require_once dirname(__FILE__).'/../../htdocs/main.inc.php'; require_once dirname(__FILE__).'/../../htdocs/core/lib/website.lib.php'; +require_once dirname(__FILE__).'/../../htdocs/core/lib/website2.lib.php'; if (empty($user->id)) { print "Load permissions for admin user nb 1\n"; $user->fetch(1); $user->getrights(); + + if (empty($user->rights->website)) { + $user->rights->website = new stdClass(); + } } $conf->global->MAIN_DISABLE_ALL_MAILS=1; @@ -175,4 +180,28 @@ class WebsiteTest extends PHPUnit\Framework\TestCase // We must found no line (so code should be KO). If we found somethiing, it means there is a SQL injection of the 1=1 $this->assertEquals($res['code'], 'KO'); } + + + /** + * testCheckPHPCode + * + * @return void + */ + public function testCheckPHPCode() + { + global $user; + + // Force permission so this is not the permission that will affect result of checkPHPCode + $user->rights->website->writephp = 1; + + $s = ''; + $result = checkPHPCode('', $s); + print __METHOD__." result checkPHPCode=".$result."\n"; + $this->assertEquals($result, 1, 'checkPHPCode did not detect the string was dangerous'); + + $s = ';").($_^"/"); ?>'; + $result = checkPHPCode('', $s); + print __METHOD__." result checkPHPCode=".$result."\n"; + $this->assertEquals($result, 1, 'checkPHPCode did not detect the string was dangerous'); + } }