N°7810 - [SECU] Portal code injection - Code Execution possible on iT…

sources/Application/TwigBase/Twig/Extension.php

@@ -14,6 +14,7 @@
 use Combodo\iTop\Renderer\BlockRenderer;
 use Dict;
 use Exception;
+use IssueLog;
 use Twig\Environment;
 use Twig\TwigFilter;
 use Twig\TwigFunction;
@@ -144,11 +145,25 @@ public static function GetFilters()
 		// Overwrite native twig filter: disable use of 'system' filter
 		$aFilters[] = new TwigFilter('filter', function (Environment $oTwigEnv, $array, $arrow) {
 			if ($arrow == 'system') {
+				IssueLog::Error('Twig "filter" filter is deactivated when used with system $arrow function');
 				return json_encode($array);
 			}
-
 			return twig_array_filter($oTwigEnv, $array, $arrow);
 		}, ['needs_environment' => true]);
+		// Since 2.7.8 deactivate map
+		$aFilters[] = new TwigFilter('map', function ($array, $arrow) {
+			IssueLog::Error('Twig "map" filter is deactivated');
+			return json_encode($array);
+		});
+		// Since 2.7.8 deactivate reduce
+		$aFilters[] = new TwigFilter('reduce', function ($array, $arrow, $initial = null) {
+			IssueLog::Error('Twig "reduce" filter is deactivated');
+			return json_encode($array);
+		});
+		$aFilters[] = new TwigFilter('sort', function ($array, $arrow, $initial = null) {
+			IssueLog::Error('Twig "sort" filter is deactivated');
+			return json_encode($array);
+		});
 
 		return $aFilters;
 	}

tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/TwigTest.php

@@ -7,51 +7,78 @@
 use Twig\Environment;
 use Twig\Loader\FilesystemLoader;
 
-class TwigTest extends ItopDataTestCase
+/**
+ * Prevent Twig from executing harmful commands (e.g. system('rm -rf')) in the Twig templates
+ * Filter function for which an argument is an "arrow function" should be secured:
+ * - either specifying a function by its name (e.g. filter('passthru'))
+ * - or using an arrow function with a safe callback (e.g. filter(v => passtrhu('ls'))
+ */
+class TwigSanitizationTest extends ItopDataTestCase
 {
+	/**
+	 * @var \Twig\Environment
+	 */
+	private Environment $oTwig;
+
+	/** @inheritdoc  */
 	protected function setUp(): void
 	{
 		parent::setUp();
 		$this->RequireOnceItopFile('core/config.class.inc.php');
-	}
 
-	/**
-	 * Test the fix for ticket N°4384
-	 *
-	 * @dataProvider TemplateProvider
-	 *
-	 */
-	public function testTemplate($sFileName, $sExpected)
-	{
 		// Creating sandbox twig env. to load and test the custom form template
-		$oTwig = new Environment(new FilesystemLoader(__DIR__.'/'));
+		$this->oTwig = new Environment(new FilesystemLoader(__DIR__));
 
 		// Manually registering filters and functions as we didn't find how to do it automatically
 		$oAppExtension = new AppExtension();
 		$aFilters = $oAppExtension->getFilters();
 		foreach ($aFilters as $oFilter)
 		{
-			$oTwig->addFilter($oFilter);
+			$this->oTwig->addFilter($oFilter);
 		}
 		$aFunctions = $oAppExtension->getFunctions();
 		foreach ($aFunctions as $oFunction)
 		{
-			$oTwig->addFunction($oFunction);
+			$this->oTwig->addFunction($oFunction);
 		}
+	}
+	private function RenderTwig(string $sTwigContent): string
+	{
+		return $this->oTwig->createTemplate($sTwigContent)->render([]);
+	}
 
-		$sOutput = $oTwig->render($sFileName);
+	public function test_reduce_FilterShouldBeDiscarded()
+	{
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|reduce('system') }}"));
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|reduce('anyCallBack') }}"));
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|reduce((carry, val, key) => carry + val) }}"));
+	}
+
+	public function test_sort_FilterShouldBeDiscarded()
+	{
+		$this->assertEquals('[3,1,2]', $this->RenderTwig("{{ [3, 1, 2]|sort('system') }}"));
+		$this->assertEquals('[3,1,2]', $this->RenderTwig("{{ [3, 1, 2]|sort('anyCallBack') }}"));
+		$this->assertEquals('[3,1,2]', $this->RenderTwig("{{ [3, 1, 2]|sort((a, b) => a > b) }}"));
 
-		$this->assertEquals($sExpected, $sOutput);
+		$this->ExpectExceptionMessage('Too few arguments to function Combodo\iTop\Application\TwigBase\Twig\Extension::Combodo\iTop\Application\TwigBase\Twig\{closure}()');
+		$this->RenderTwig("{{ [3, 1, 2]|sort|join(', ') }}");
 	}
 
-	public static function TemplateProvider()
+	public function test_map_FilterShouldBeDiscarded()
 	{
-		$aReturn = array();
-		$aReturn['filter_system'] = [
-				'sFileName' => 'test.html',
-				'expected' => file_get_contents(__DIR__.'/test.html'),
-			];
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|map('system') }}"));
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|map('anyCallBack') }}"));
+		$this->assertEquals('[1,2,3]', $this->RenderTwig("{{ [1, 2, 3]|map(p => p + 10) }}"));
+	}
+
+	public function test_filter_FilterShouldNotAllowTheSystemFunction()
+	{
+		$this->assertEquals('["ls"]', $this->RenderTwig("{{ ['ls']|filter('system')|raw }}"), 'system() should not be allowed as callback for filter');
+
+		$this->assertEquals('Iterator', $this->RenderTwig("{{ ['Iterator', 'Zabugomeu']|filter('interface_exists')|join(', ') }}"), 'Other functions should be allowed as callback for filter');
+		$this->assertEquals('4, 5', $this->RenderTwig("{{ [1, 2, 3, 4, 5]|filter(v => v > 3)|join(', ') }}"), 'Arrow functions should be allowed as callback');
 
-		return $aReturn;
+		$this->ExpectExceptionMessage('Unknown "system" function', 'system() should not be allowed in arrow functions');
+		$this->RenderTwig("{{ [1, 2, 3, 4, 5]|filter(v => v > system('ls >ls.txt'))|join(', ') }}");
 	}
 }