Repository: twigphp/Twig (php)

Issue:
Regression: GHSA-6377-hfv9-hqf6 fix can exceed recursion limits / OOM
Several of us who updated twig/twig:3.14.1 on our Drupal web sites have seen that this new recursive logic causes OOM or stack limits. This seems related to [https://github.com/twigphp/Twig/security/advisories/GHSA-6377-hfv9-hqf6](GHSA-6377-hfv9-hqf6).

With xdebug enable, this is displayed as


    Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '1000' frames in Twig\Extension\SandboxExtension->isSandboxed() (line 56 of /var/www/html/vendor/twig/twig/src/Extension/SandboxExtension.php).
    
    Twig\Extension\SandboxExtension->ensureToStringAllowed() (Line: 124)
    Twig\Extension\SandboxExtension->ensureToStringAllowed() (Line: 93)


Drupal form render arrays are often very nested and very deep. I do not quite understand the twig sandbox code so I do not know if this is fixable downstream, but it seems like we would only need to recurse if the value is an object rather than on every array/list value.

Any help would be appreciated.

**Steps to reproduce**

1. Install Drupal 10.3 or >11.0.1 (helpful to use Standard profile) 
2. Update to twig/twig:3.14.1
3. Log in as an administrative user.
4. Attempt to create a node.


[drupal.org issue](https://www.drupal.org/project/drupal/issues/3485956)


Pull request description:
Add recursion prevention
Resolves #4439

Drupal allows printing arrays direct in templates, as we have a node visitor/render function that can 'render' our 'render arrays'.

This adds some recursion prevention.

Patch:
diff --git a/doc/api.rst b/doc/api.rst
index d45af362..8da69503 100644
--- a/doc/api.rst
+++ b/doc/api.rst
@@ -488,7 +488,7 @@ won't be allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception.
 
 .. note::
 
-    As of Twig 1.14.1 (and on Twig 3.11.2), if the ``Article`` class implements
+    As of Twig 3.14.1 (and on Twig 3.11.2), if the ``Article`` class implements
     the ``ArrayAccess`` interface, the templates will only be able to access
     the ``title`` and ``body`` attributes.
 
diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php
index 95b6295a..255f2f28 100644
--- a/src/Extension/SandboxExtension.php
+++ b/src/Extension/SandboxExtension.php
@@ -120,9 +120,7 @@ final class SandboxExtension extends AbstractExtension
     public function ensureToStringAllowed($obj, int $lineno = -1, ?Source $source = null)
     {
         if (\is_array($obj)) {
-            foreach ($obj as $v) {
-                $this->ensureToStringAllowed($v, $lineno, $source);
-            }
+            $this->ensureToStringAllowedForArray($obj, $lineno, $source);
 
             return $obj;
         }
@@ -140,4 +138,45 @@ final class SandboxExtension extends AbstractExtension
 
         return $obj;
     }
+
+    private function ensureToStringAllowedForArray(array $obj, int $lineno, ?Source $source, array &$stack = []): void
+    {
+        foreach ($obj as $k => $v) {
+            if (!$v) {
+                continue;
+            }
+
+            if (!\is_array($v)) {
+                $this->ensureToStringAllowed($v, $lineno, $source);
+                continue;
+            }
+
+            if (\PHP_VERSION_ID < 70400) {
+                static $cookie;
+
+                if ($v === $cookie ?? $cookie = new \stdClass()) {
+                    continue;
+                }
+
+                $obj[$k] = $cookie;
+                try {
+                    $this->ensureToStringAllowedForArray($v, $lineno, $source, $stack);
+                } finally {
+                    $obj[$k] = $v;
+                }
+
+                continue;
+            }
+
+            if ($r = \ReflectionReference::fromArrayElement($obj, $k)) {
+                if (isset($stack[$r->getId()])) {
+                    continue;
+                }
+
+                $stack[$r->getId()] = true;
+            }
+
+            $this->ensureToStringAllowedForArray($v, $lineno, $source, $stack);
+        }
+    }
 }


Test patch:
diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
index cc74e8cd..647ea295 100644
--- a/tests/Extension/SandboxTest.php
+++ b/tests/Extension/SandboxTest.php
@@ -41,7 +41,10 @@ class SandboxTest extends TestCase
             'some_array' => [5, 6, 7, new FooObject()],
             'array_like' => new ArrayLikeObject(),
             'magic' => new MagicObject(),
+            'recursion' => [4],
         ];
+        self::$params['recursion'][] = &self::$params['recursion'];
+        self::$params['recursion'][] = new FooObject();
 
         self::$templates = [
             '1_basic1' => '{{ obj.foo }}',
@@ -240,6 +243,7 @@ class SandboxTest extends TestCase
             'context' => ['{{ _context|join(", ") }}'],
             'spread_array_operator' => ['{{ [1, 2, ...[5, 6, 7, obj]]|join(",") }}'],
             'spread_array_operator_var' => ['{{ [1, 2, ...some_array]|join(",") }}'],
+            'recursion' => ['{{ recursion|join(", ") }}'],
         ];
     }
 
@@ -573,13 +577,13 @@ class ArrayLikeObject extends \ArrayObject
 {
     public function offsetExists($offset): bool
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 
     #[\ReturnTypeWillChange]
     public function offsetGet($offset)
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 
     public function offsetSet($offset, $value): void
@@ -596,11 +600,11 @@ class MagicObject
     #[\ReturnTypeWillChange]
     public function __get($name)
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 
     public function __isset($name): bool
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 }