CVE-2022-23614: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: Twig is an open source template language for PHP. When in a sandbox mode, the `arrow` parameter of the `sort` filter must be a closure to avoid attackers being able to run arbitrary PHP functions. In affected versions this constraint was not properly enforced and could lead to code injection of arbitrary PHP code. Patched versions now disallow calling non Closure in the `sort` filter as is the case for some other filters. Users are advised to upgrade.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 9.8
Published: 2022-02-04T23:15Z
Repository: https://github.com/twigphp/Twig
Language: PHP
Files changed: CHANGELOG, src/Extension/CoreExtension.php, tests/Extension/SandboxTest.php
Commit date: 2022-02-04 07:52:21 +0100
Commit message: Disallow non closures in `sort` filter when the sanbox mode is enabled

Diff: diff --git a/CHANGELOG b/CHANGELOG
index 95496ba3c..3aaa9aac0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,12 +1,12 @@
-# 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)
 
 * Allow null when Twig expects a string (for better 8.1 support)
 * Add support for PHP 7.1 back
diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php
index e94a900e9..6ac361081 100644
--- a/src/Extension/CoreExtension.php
+++ b/src/Extension/CoreExtension.php
@@ -235,11 +235,11 @@ final class CoreExtension extends AbstractExtension
             new TwigFilter('spaceless', 'twig_spaceless', ['is_safe' => ['html']]),
 
             // 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'),
             new TwigFilter('filter', 'twig_array_filter', ['needs_environment' => true]),
             new TwigFilter('map', 'twig_array_map', ['needs_environment' => true]),
@@ -924,19 +924,21 @@ function twig_reverse_filter(Environment $env, $item, $preserveKeys = false)
  *
  * @param array|\Traversable $array
  *
  * @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);
     } elseif (!\is_array($array)) {
         throw new RuntimeError(sprintf('The sort filter only works with arrays or "Traversable", got "%s".', \gettype($array)));
     }
 
     if (null !== $arrow) {
+        twig_check_arrow_in_sandbox($env, $arrow, 'sort', 'filter');
+
         uasort($array, $arrow);
     } else {
         asort($array);
     }
 
@@ -1604,13 +1606,11 @@ function twig_array_filter(Environment $env, $array, $arrow)
 {
     if (!twig_test_iterable($array)) {
         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);
     }
 
@@ -1618,13 +1618,11 @@ function twig_array_filter(Environment $env, $array, $arrow)
     return new \CallbackFilterIterator(new \IteratorIterator($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) {
         $r[$k] = $arrow($v, $k);
     }
@@ -1632,13 +1630,11 @@ function twig_array_map(Environment $env, $array, $arrow)
     return $r;
 }
 
 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) {
             throw new RuntimeError(sprintf('The "reduce" filter only works with arrays or "Traversable", got "%s" as first argument.', \gettype($array)));
         }
@@ -1646,6 +1642,13 @@ function twig_array_reduce(Environment $env, $array, $arrow, $initial = null)
         $array = iterator_to_array($array);
     }
 
     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));
+    }
+}
 }
diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
index 0d9bc0afe..e365da632 100644
--- a/tests/Extension/SandboxTest.php
+++ b/tests/Extension/SandboxTest.php
@@ -388,11 +388,11 @@ EOF
     }
 
     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(", ") }}
 EOF
         ], [], ['escape', 'filter', 'join']);

Vulnerable code: # 2.14.11 (2022-XX-XX)
* n/a
* Allow more null arguments when Twig expects a string (for better 8.1 support)
            new TwigFilter('sort', 'twig_sort_filter'),
function twig_sort_filter($array, $arrow = null)
    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.');
    }
    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.');
    }
    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.');
    }
        $this->expectExceptionMessage('The callable passed to "filter" filter must be a Closure in sandbox mode in "index" at line 1.');
Fixed code: # 2.14.11 (2022-02-04)
 * Fix a security issue when in a sandbox: the `sort` filter must require a Closure for the `arrow` parameter
 * Allow more null arguments when Twig expects a string (for better 8.1 support)
            new TwigFilter('sort', 'twig_sort_filter', ['needs_environment' => true]),
function twig_sort_filter(Environment $env, $array, $arrow = null)
        twig_check_arrow_in_sandbox($env, $arrow, 'sort', 'filter');

    twig_check_arrow_in_sandbox($env, $arrow, 'filter', 'filter');
    twig_check_arrow_in_sandbox($env, $arrow, 'map', 'filter');
    twig_check_arrow_in_sandbox($env, $arrow, 'reduce', 'filter');

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));
    }
}
        $this->expectExceptionMessage('The callable passed to the "filter" filter must be a Closure in sandbox mode in "index" at line 1.');