Merge branch '3.11.x' into 3.14.x

* 3.11.x: Improve detection of recursion Fix recursion when arrays contain self-references in sandboxed mode Fix code Prepare the 3.11.2 release Update CHANGELOG Sandbox ArrayAccess and do sandbox checks before isset() checks Fix sandbox handling for __toString() Prepare the 3.11.1 release Fix a security issue when an included sandboxed template has been loaded before without the sandbox context
twigphp · Nov 7, 2024 · 83a21d3 · 83a21d3
2 parents f405356 + d3fc074
commit 83a21d3
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 10 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,6 @@
 # 3.14.1 (2024-11-06)
 
- * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects
+ * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects
    They are now checked via the property policy
  * Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()`
    under some circumstances on an object even if the `__toString()` method is not allowed by the security policy
@@ -51,6 +51,17 @@
  * Fix integration tests when a test has more than one data/expect section and deprecations
  * Add the `enum_cases` function
 
+# 3.11.2 (2024-11-06)
+
+ * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects
+   They are now checked via the property policy
+ * Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()`
+   under some circumstances on an object even if the `__toString()` method is not allowed by the security policy
+
+# 3.11.1 (2024-09-10)
+
+ * Fix a security issue when an included sandboxed template has been loaded before without the sandbox context
+
 # 3.11.0 (2024-08-08)
 
  * Deprecate `OptimizerNodeVisitor::OPTIMIZE_RAW_FILTER`

diff --git 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
@@ -120,14 +120,12 @@ public function checkPropertyAllowed($obj, $property, int $lineno = -1, ?Source
     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;
         }
 
-        if ($this->isSandboxed($source) && $obj instanceof \Stringable) {
+        if ($obj instanceof \Stringable && $this->isSandboxed($source)) {
             try {
                 $this->policy->checkMethodAllowed($obj, '__toString');
             } catch (SecurityNotAllowedMethodError $e) {
@@ -140,4 +138,28 @@ public function ensureToStringAllowed($obj, int $lineno = -1, ?Source $source =
 
         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 ($r = \ReflectionReference::fromArrayElement($obj, $k)) {
+                if (isset($stack[$r->getId()])) {
+                    continue;
+                }
+
+                $stack[$r->getId()] = true;
+            }
+
+            $this->ensureToStringAllowedForArray($v, $lineno, $source, $stack);
+        }
+    }
 }
diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
@@ -45,7 +45,10 @@ protected function setUp(): void
             '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 }}',
@@ -302,6 +305,7 @@ public static function getSandboxUnallowedToStringTests()
             '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(", ") }}'],
         ];
     }
 
@@ -635,12 +639,12 @@ class ArrayLikeObject extends \ArrayObject
 {
     public function offsetExists($offset): bool
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 
     public function offsetGet($offset): mixed
     {
-        throw new \BadMethodCallException('Should not be called');
+        throw new \BadMethodCallException('Should not be called.');
     }
 
     public function offsetSet($offset, $value): void
@@ -656,11 +660,11 @@ class MagicObject
 {
     public function __get($name): mixed
     {
-        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.');
     }
 }