diff --git a/sources/Application/TwigBase/Twig/Extension.php b/sources/Application/TwigBase/Twig/Extension.php
index 7c9dd59f27..627ee208f5 100644
--- a/sources/Application/TwigBase/Twig/Extension.php
+++ b/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;
}
diff --git a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/TwigTest.php b/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/TwigTest.php
index a460a88fa6..dd05edcd4b 100644
--- a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/TwigTest.php
+++ b/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(', ') }}");
}
}
\ No newline at end of file
diff --git a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/attacker/backdoor b/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/attacker/backdoor
deleted file mode 100644
index f56435e7a2..0000000000
--- a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/attacker/backdoor
+++ /dev/null
@@ -1 +0,0 @@
-!!! BACKDOOR !!!
\ No newline at end of file
diff --git a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html b/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html
deleted file mode 100644
index f63b8a51a3..0000000000
--- a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html
+++ /dev/null
@@ -1,46 +0,0 @@
-User Name
-
-['id']|filter('system')|join
-["id"]
-
-['echo']|filter('passthru')|join
-["echo"]
-
-['echo']|filter('popen')|join
-["echo"]
-
-['echo']|filter('exec')|join
-["echo"]
-
-['id']|filter('SysteM')|join
-["id"]
-
-['touch+/tmp/test+']|filter('system')|join(',')
-["touch+\/tmp\/test+"]
-
-[34, 36, 38, 40, 42]|filter(v => v > 38)|join(', ')
-[34,36,38,40,42]
-
-app.request.server.all|join(',')
-
-
-self
-
-
-[0]|reduce('system','echo')
-0
-
-[1, 2, 3]|reduce((carry, v) => carry + v)
-1, 2, 3
-
-['echo']|map('system')|join
-echo
-
-{"Bob": "Smith", "Alice": "Dupond"}|map((value, key) => "#{key} #{value}")|join(', ')
-Smith, Dupond
-
-['echo',1]|sort('system')|join
-["echo",1]
-
-POST /subscribe?0=cat+/etc/passwd HTTP/1.1
-email=""@attacker.tld
\ No newline at end of file
diff --git a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html.twig b/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html.twig
deleted file mode 100644
index 1232f8104b..0000000000
--- a/tests/php-unit-tests/unitary-tests/sources/Application/TwigBase/Twig/test.html.twig
+++ /dev/null
@@ -1,51 +0,0 @@
-{{ 'UI:Login:UserNamePrompt'|dict_s }}
-
-['id']|filter('system')|join
-{{ ['id']|filter('system')|join }}
-
-['echo']|filter('passthru')|join
-{{ ['echo']|filter('passthru')|join }}
-
-['echo']|filter('popen')|join
-{{ ['echo']|filter('popen')|join }}
-
-['echo']|filter('exec')|join
-{{ ['echo']|filter('exec')|join }}
-
-['id']|filter('SysteM')|join
-{{ ['id']|filter('SysteM')|join }}
-
-['touch+/tmp/test+']|filter('system')|join(',')
-{{ ['touch+/tmp/test+']|filter('system')|join(',') }}
-
-[34, 36, 38, 40, 42]|filter(v => v > 38)|join(', ')
-{{ [34, 36, 38, 40, 42]|filter(v => v > 38)|join(', ') }}
-
-app.request.server.all|join(',')
-{{ app.request.server.all|join(',')}}
-
-self
-{{ self }}
-
-[0]|reduce('system','echo')
-{{ [0]|reduce('system','echo')|join(', ') }}
-
-[1, 2, 3]|reduce((carry, v) => carry + v)
-{% set numbers = [1, 2, 3] %}
-{{ numbers|reduce((carry, v) => carry + v)|join(', ') }}
-
-['echo']|map('system')|join
-{{ ['echo']|map('system')|join }}
-
-{"Bob": "Smith", "Alice": "Dupond"}|map((value, key) => "#{key} #{value}")|join(', ')
-{% set people = {
-"Bob": "Smith",
-"Alice": "Dupond",
-} %}
-{{ people|map((value, key) => "#{key} #{value}")|join(', ') }}
-
-['echo',1]|sort('system')|join
-{{ ['echo',1]|sort('system')|join }}
-
-POST /subscribe?0=cat+/etc/passwd HTTP/1.1
-email="{{ app.request.query.filter(0,0,1024,{'options':'system'}) }}"@attacker.tld
\ No newline at end of file