bug #3641 Disallow non closures in sort filter when the sanbox mode is enabled (fabpot)

This PR was merged into the 2.x branch.

Discussion
----------

Disallow non closures in `sort` filter when the sanbox mode is enabled

Commits
-------

2eb3308 Disallow non closures in `sort` filter when the sanbox mode is enabled

Author: fabpot

CHANGELOG

@@ -1,10 +1,10 @@
-# 2.14.11 (2022-XX-XX)
+# 2.14.11 (2022-02-04)
 
-* n/a
+ * Fix a security issue when in a sandbox: the `sort` filter must require a Closure for the `arrow` parameter
 
 # 2.14.10 (2022-01-03)
 
-* Allow more null arguments when Twig expects a string (for better 8.1 support)
+ * Allow more null arguments when Twig expects a string (for better 8.1 support)
 
 # 2.14.9 (2022-01-03)
 

src/Extension/CoreExtension.php

@@ -237,7 +237,7 @@ public function getFilters()
             // array helpers
             new TwigFilter('join', 'twig_join_filter'),
             new TwigFilter('split', 'twig_split_filter', ['needs_environment' => true]),
-            new TwigFilter('sort', 'twig_sort_filter'),
+            new TwigFilter('sort', 'twig_sort_filter', ['needs_environment' => true]),
             new TwigFilter('merge', 'twig_array_merge'),
             new TwigFilter('batch', 'twig_array_batch'),
             new TwigFilter('column', 'twig_array_column'),
@@ -926,7 +926,7 @@ function twig_reverse_filter(Environment $env, $item, $preserveKeys = false)
  *
  * @return array
  */
-function twig_sort_filter($array, $arrow = null)
+function twig_sort_filter(Environment $env, $array, $arrow = null)
 {
     if ($array instanceof \Traversable) {
         $array = iterator_to_array($array);
@@ -935,6 +935,8 @@ function twig_sort_filter($array, $arrow = null)
     }
 
     if (null !== $arrow) {
+        twig_check_arrow_in_sandbox($env, $arrow, 'sort', 'filter');
+
         uasort($array, $arrow);
     } else {
         asort($array);
@@ -1606,9 +1608,7 @@ function twig_array_filter(Environment $env, $array, $arrow)
         throw new RuntimeError(sprintf('The "filter" filter expects an array or "Traversable", got "%s".', \is_object($array) ? \get_class($array) : \gettype($array)));
     }
 
-    if (!$arrow instanceof Closure && $env->hasExtension('\Twig\Extension\SandboxExtension') && $env->getExtension('\Twig\Extension\SandboxExtension')->isSandboxed()) {
-        throw new RuntimeError('The callable passed to "filter" filter must be a Closure in sandbox mode.');
-    }
+    twig_check_arrow_in_sandbox($env, $arrow, 'filter', 'filter');
 
     if (\is_array($array)) {
         return array_filter($array, $arrow, \ARRAY_FILTER_USE_BOTH);
@@ -1620,9 +1620,7 @@ function twig_array_filter(Environment $env, $array, $arrow)
 
 function twig_array_map(Environment $env, $array, $arrow)
 {
-    if (!$arrow instanceof Closure && $env->hasExtension('\Twig\Extension\SandboxExtension') && $env->getExtension('\Twig\Extension\SandboxExtension')->isSandboxed()) {
-        throw new RuntimeError('The callable passed to the "map" filter must be a Closure in sandbox mode.');
-    }
+    twig_check_arrow_in_sandbox($env, $arrow, 'map', 'filter');
 
     $r = [];
     foreach ($array as $k => $v) {
@@ -1634,9 +1632,7 @@ function twig_array_map(Environment $env, $array, $arrow)
 
 function twig_array_reduce(Environment $env, $array, $arrow, $initial = null)
 {
-    if (!$arrow instanceof Closure && $env->hasExtension('\Twig\Extension\SandboxExtension') && $env->getExtension('\Twig\Extension\SandboxExtension')->isSandboxed()) {
-        throw new RuntimeError('The callable passed to the "reduce" filter must be a Closure in sandbox mode.');
-    }
+    twig_check_arrow_in_sandbox($env, $arrow, 'reduce', 'filter');
 
     if (!\is_array($array)) {
         if (!$array instanceof \Traversable) {
@@ -1648,4 +1644,11 @@ function twig_array_reduce(Environment $env, $array, $arrow, $initial = null)
 
     return array_reduce($array, $arrow, $initial);
 }
+
+function twig_check_arrow_in_sandbox(Environment $env, $arrow, $thing, $type)
+{
+    if (!$arrow instanceof Closure && $env->hasExtension('\Twig\Extension\SandboxExtension') && $env->getExtension('\Twig\Extension\SandboxExtension')->isSandboxed()) {
+        throw new RuntimeError(sprintf('The callable passed to the "%s" %s must be a Closure in sandbox mode.', $thing, $type));
+    }
+}
 }

tests/Extension/SandboxTest.php

@@ -390,7 +390,7 @@ public function testSandboxDisabledAfterIncludeFunctionError()
     public function testSandboxWithNoClosureFilter()
     {
         $this->expectException('\Twig\Error\RuntimeError');
-        $this->expectExceptionMessage('The callable passed to "filter" filter must be a Closure in sandbox mode in "index" at line 1.');
+        $this->expectExceptionMessage('The callable passed to the "filter" filter must be a Closure in sandbox mode in "index" at line 1.');
 
         $twig = $this->getEnvironment(true, ['autoescape' => 'html'], ['index' => <<<EOF
 {{ ["foo", "bar", ""]|filter("trim")|join(", ") }}