Browse Source

FIX #huntr5affff95-9a37-4004-bab2-a834b3b61ff7

Laurent Destailleur 2 years ago
parent
commit
81ef87cf6d
2 changed files with 36 additions and 1 deletions
  1. 7 1
      htdocs/core/lib/website2.lib.php
  2. 29 0
      test/phpunit/Website.class.php

+ 7 - 1
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;
 			}

+ 29 - 0
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 = '<?php exec("eee"); ?>';
+		$result = checkPHPCode('', $s);
+		print __METHOD__." result checkPHPCode=".$result."\n";
+		$this->assertEquals($result, 1, 'checkPHPCode did not detect the string was dangerous');
+
+		$s = '<?php $_="{"; $_=($_^"<").($_^">;").($_^"/"); ?><?=${\'_\'.$_}["_"](${\'_\'.$_}["__"]);?>';
+		$result = checkPHPCode('', $s);
+		print __METHOD__." result checkPHPCode=".$result."\n";
+		$this->assertEquals($result, 1, 'checkPHPCode did not detect the string was dangerous');
+	}
 }