Merge pull request #18208 from craftcms/feature/disallow-string-arrows

Disallow non-Closure arrow functions

Author: brandonkelly

src/config/GeneralConfig.php

@@ -1224,6 +1224,24 @@ class GeneralConfig extends BaseConfig
      */
     public bool $enableTemplateCaching = true;
 
+    /**
+     * @var bool Whether all Twig templates should be sandboxed.
+     *
+     * ::: code
+     * ```php Static Config
+     * ->enableTwigSandbox(false)
+     * ```
+     * ```shell Environment Override
+     * CRAFT_ENABLE_TWIG_SANDBOX=false
+     * ```
+     * :::
+     *
+     * @see enableTwigSandbox()
+     * @group Security
+     * @since 4.17.0
+     */
+    public bool $enableTwigSandbox = false;
+
     /**
      * @var string The prefix that should be prepended to HTTP error status codes when determining the path to look for an error’s template.
      *
@@ -4564,6 +4582,25 @@ public function enableTemplateCaching(bool $value = true): self
         return $this;
     }
 
+    /**
+     * Whether all Twig templates should be sandboxed.
+     *
+     * ```php
+     * ->enableTwigSandbox(false)
+     * ```
+     *
+     * @group Security
+     * @param bool $value
+     * @return self
+     * @see $enableTwigSandbox
+     * @since 4.17.0
+     */
+    public function enableTwigSandbox(bool $value = true): self
+    {
+        $this->enableTwigSandbox = $value;
+        return $this;
+    }
+
     /**
      * The prefix that should be prepended to HTTP error status codes when determining the path to look for an error’s template.
      *

src/web/View.php

@@ -25,6 +25,7 @@
 use craft\web\twig\Extension;
 use craft\web\twig\FeExtension;
 use craft\web\twig\GlobalsExtension;
+use craft\web\twig\SecurityPolicy;
 use craft\web\twig\SinglePreloaderExtension;
 use craft\web\twig\TemplateLoader;
 use Illuminate\Support\Collection;
@@ -35,6 +36,7 @@
 use Twig\Error\SyntaxError as TwigSyntaxError;
 use Twig\Extension\CoreExtension;
 use Twig\Extension\ExtensionInterface;
+use Twig\Extension\SandboxExtension;
 use Twig\Extension\StringLoaderExtension;
 use Twig\Template as TwigTemplate;
 use Twig\TemplateWrapper;
@@ -395,6 +397,9 @@ public function createTwig(): Environment
 
         $twig = new Environment(new TemplateLoader($this), $this->_getTwigOptions());
 
+        // Even an empty security policy will prevent non-closures from being allowed as arrow functions
+        $twig->addExtension(new SandboxExtension(new SecurityPolicy(), Craft::$app->getConfig()->getGeneral()->enableTwigSandbox));
+
         $twig->addExtension(new StringLoaderExtension());
         $twig->addExtension(new Extension($this, $twig));
 

src/web/twig/Extension.php

@@ -99,7 +99,7 @@ class Extension extends AbstractExtension implements GlobalsInterface
      */
     public static function arraySome(TwigEnvironment $env, $array, $arrow)
     {
-        self::checkArrowFunction($arrow, 'has some', 'operator');
+        CoreExtension::checkArrow($env, $arrow, 'has some', 'operator');
         return CoreExtension::arraySome($env, $array, $arrow);
     }
 
@@ -108,38 +108,10 @@ public static function arraySome(TwigEnvironment $env, $array, $arrow)
      */
     public static function arrayEvery(TwigEnvironment $env, $array, $arrow)
     {
-        self::checkArrowFunction($arrow, 'has every', 'operator');
+        CoreExtension::checkArrow($env, $arrow, 'has every', 'operator');
         return CoreExtension::arrayEvery($env, $array, $arrow);
     }
 
-    /**
-     * Called by:
-     * - has every (operator)
-     * - has some (operator)
-     * - |filter
-     * - |find
-     * - |map
-     * - |reduce
-     * - |sort
-     */
-    private static function checkArrowFunction(mixed $arrow, string $thing, string $type): void
-    {
-        if (
-            is_string($arrow) &&
-            in_array(ltrim(strtolower($arrow), '\\'), [
-                'system',
-                'passthru',
-                'exec',
-                'file_get_contents',
-                'file_put_contents',
-                'popen',
-                'call_user_func',
-            ])
-        ) {
-            throw new RuntimeError(sprintf('The "%s" %s does not support passing "%s".', $thing, $type, $arrow));
-        }
-    }
-
     /**
      * @var View|null
      */
@@ -253,7 +225,7 @@ public function getFilters(): array
             new TwigFilter('filesize', [$this, 'filesizeFilter']),
             new TwigFilter('filter', [$this, 'filterFilter'], ['needs_environment' => true]),
             new TwigFilter('filterByValue', [ArrayHelper::class, 'where'], ['deprecation_info' => new DeprecatedCallableInfo('craftcms/cms', '3.5.0', 'where')]),
-            new TwigFilter('group', [$this, 'groupFilter']),
+            new TwigFilter('group', [$this, 'groupFilter'], ['needs_environment' => true]),
             new TwigFilter('hash', [$security, 'hashData']),
             new TwigFilter('httpdate', [$this, 'httpdateFilter'], ['needs_environment' => true]),
             new TwigFilter('id', [Html::class, 'id']),
@@ -565,7 +537,7 @@ public function snakeFilter(mixed $string): string
      */
     public function sortFilter(TwigEnvironment $env, iterable $array, string|callable|null $arrow = null): array
     {
-        self::checkArrowFunction($arrow, 'sort', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'sort', 'filter');
         return CoreExtension::sort($env, $array, $arrow);
     }
 
@@ -582,7 +554,7 @@ public function sortFilter(TwigEnvironment $env, iterable $array, string|callabl
      */
     public function reduceFilter(TwigEnvironment $env, mixed $array, mixed $arrow, mixed $initial = null): mixed
     {
-        self::checkArrowFunction($arrow, 'reduce', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'reduce', 'filter');
         return CoreExtension::reduce($env, $array, $arrow, $initial);
     }
 
@@ -598,7 +570,7 @@ public function reduceFilter(TwigEnvironment $env, mixed $array, mixed $arrow, m
      */
     public function mapFilter(TwigEnvironment $env, mixed $array, mixed $arrow = null): array
     {
-        self::checkArrowFunction($arrow, 'map', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'map', 'filter');
         return CoreExtension::map($env, $array, $arrow);
     }
 
@@ -723,7 +695,7 @@ public function timestampFilter(mixed $value, ?string $format = null, bool $with
      */
     public function findFilter(TwigEnvironment $env, $array, $arrow): mixed
     {
-        self::checkArrowFunction($arrow, 'find', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'find', 'filter');
         return CoreExtension::find($env, $array, $arrow);
     }
 
@@ -1202,7 +1174,7 @@ public function encencFilter(mixed $str): string
      */
     public function filterFilter(TwigEnvironment $env, iterable $arr, ?callable $arrow = null): array
     {
-        self::checkArrowFunction($arrow, 'filter', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'filter', 'filter');
 
         /** @var array|Traversable $arr */
         if ($arrow === null) {
@@ -1224,14 +1196,15 @@ public function filterFilter(TwigEnvironment $env, iterable $arr, ?callable $arr
     /**
      * Groups an array by the results of an arrow function, or value of a property.
      *
+     * @param TwigEnvironment $env
      * @param iterable $arr
      * @param callable|string $arrow The arrow function or property name that determines the group the item should be grouped in
      * @return array[] The grouped items
      * @throws RuntimeError if $arr is not of type array or Traversable
      */
-    public function groupFilter(iterable $arr, callable|string $arrow): array
+    public function groupFilter(TwigEnvironment $env, iterable $arr, callable|string $arrow): array
     {
-        self::checkArrowFunction($arrow, 'group', 'filter');
+        CoreExtension::checkArrow($env, $arrow, 'group', 'filter');
 
         $groups = [];
 

src/web/twig/SecurityPolicy.php

@@ -0,0 +1,31 @@
+<?php
+/**
+ * @link https://craftcms.com/
+ * @copyright Copyright (c) Pixel & Tonic, Inc.
+ * @license https://craftcms.github.io/license/
+ */
+
+namespace craft\web\twig;
+
+use Twig\Sandbox\SecurityPolicyInterface;
+
+/**
+ * Security policy
+ *
+ * @author Pixel & Tonic, Inc. <[email protected]>
+ * @since 4.17.0
+ */
+class SecurityPolicy implements SecurityPolicyInterface
+{
+    public function checkSecurity($tags, $filters, $functions): void
+    {
+    }
+
+    public function checkMethodAllowed($obj, $method): void
+    {
+    }
+
+    public function checkPropertyAllowed($obj, $property): void
+    {
+    }
+}