From 2bb8c2460a2c519c498df9b643d5277117155a73 Mon Sep 17 00:00:00 2001
From: Fabien Potencier <fabien@potencier.org>
Date: Fri, 25 Oct 2024 11:04:18 +0200
Subject: [PATCH] Fix sandbox handling for __toString()

---
 src/Extension/SandboxExtension.php     |  8 ++++++++
 src/NodeVisitor/SandboxNodeVisitor.php | 15 ++++++++++++++-
 tests/Extension/SandboxTest.php        | 15 +++++++++++++--
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php
index 4e96760f7d4..c9ffe6477bd 100644
--- a/src/Extension/SandboxExtension.php
+++ b/src/Extension/SandboxExtension.php
@@ -119,6 +119,14 @@ 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);
+            }
+
+            return $obj;
+        }
+
         if ($this->isSandboxed($source) && $obj instanceof \Stringable) {
             try {
                 $this->policy->checkMethodAllowed($obj, '__toString');
diff --git a/src/NodeVisitor/SandboxNodeVisitor.php b/src/NodeVisitor/SandboxNodeVisitor.php
index 37e184a3edc..8c15db0d6f2 100644
--- a/src/NodeVisitor/SandboxNodeVisitor.php
+++ b/src/NodeVisitor/SandboxNodeVisitor.php
@@ -15,12 +15,14 @@
 use Twig\Node\CheckSecurityCallNode;
 use Twig\Node\CheckSecurityNode;
 use Twig\Node\CheckToStringNode;
+use Twig\Node\Expression\ArrayExpression;
 use Twig\Node\Expression\Binary\ConcatBinary;
 use Twig\Node\Expression\Binary\RangeBinary;
 use Twig\Node\Expression\FilterExpression;
 use Twig\Node\Expression\FunctionExpression;
 use Twig\Node\Expression\GetAttrExpression;
 use Twig\Node\Expression\NameExpression;
+use Twig\Node\Expression\Unary\SpreadUnary;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
 use Twig\Node\PrintNode;
@@ -120,7 +122,18 @@ private function wrapNode(Node $node, string $name): void
     {
         $expr = $node->getNode($name);
         if (($expr instanceof NameExpression || $expr instanceof GetAttrExpression) && !$expr->isGenerator()) {
-            $node->setNode($name, new CheckToStringNode($expr));
+            // Simplify in 4.0 as the spread attribute has been removed there
+            $new = new CheckToStringNode($expr);
+            if ($expr->hasAttribute('spread')) {
+                $new->setAttribute('spread', $expr->getAttribute('spread'));
+            }
+            $node->setNode($name, $new);
+        } elseif ($expr instanceof SpreadUnary) {
+            $this->wrapNode($expr, 'node');
+        } elseif ($expr instanceof ArrayExpression) {
+            foreach ($expr as $name => $_) {
+                $this->wrapNode($expr, $name);
+            }
         }
     }
 
diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
index d24a06c6720..59e68f67ec2 100644
--- a/tests/Extension/SandboxTest.php
+++ b/tests/Extension/SandboxTest.php
@@ -42,6 +42,7 @@ protected function setUp(): void
             'obj' => new FooObject(),
             'arr' => ['obj' => new FooObject()],
             'child_obj' => new ChildClass(),
+            'some_array' => [5, 6, 7, new FooObject()],
         ];
 
         self::$templates = [
@@ -246,10 +247,10 @@ public function testSandboxUnallowedProperty()
      */
     public function testSandboxUnallowedToString($template)
     {
-        $twig = $this->getEnvironment(true, [], ['index' => $template], [], ['upper'], ['Twig\Tests\Extension\FooObject' => 'getAnotherFooObject'], [], ['random']);
+        $twig = $this->getEnvironment(true, [], ['index' => $template], [], ['upper', 'join', 'replace'], ['Twig\Tests\Extension\FooObject' => 'getAnotherFooObject'], [], ['random']);
         try {
             $twig->load('index')->render(self::$params);
-            $this->fail('Sandbox throws a SecurityError exception if an unallowed method (__toString()) is called in the template');
+            $this->fail('Sandbox throws a SecurityError exception if an unallowed method "__toString()" method is called in the template');
         } catch (SecurityNotAllowedMethodError $e) {
             $this->assertEquals('Twig\Tests\Extension\FooObject', $e->getClassName(), 'Exception should be raised on the "Twig\Tests\Extension\FooObject" class');
             $this->assertEquals('__tostring', $e->getMethodName(), 'Exception should be raised on the "__toString" method');
@@ -272,6 +273,16 @@ public static function getSandboxUnallowedToStringTests()
             'object_chain_and_function' => ['{{ random(obj.anotherFooObject) }}'],
             'concat' => ['{{ obj ~ "" }}'],
             'concat_again' => ['{{ "" ~ obj }}'],
+            'object_in_arguments' => ['{{ "__toString"|replace({"__toString": obj}) }}'],
+            'object_in_array' => ['{{ [12, "foo", obj]|join(", ") }}'],
+            'object_in_array_var' => ['{{ some_array|join(", ") }}'],
+            'object_in_array_nested' => ['{{ [12, "foo", [12, "foo", obj]]|join(", ") }}'],
+            'object_in_array_var_nested' => ['{{ [12, "foo", some_array]|join(", ") }}'],
+            'object_in_array_dynamic_key' => ['{{ {(obj): "foo"}|join(", ") }}'],
+            'object_in_array_dynamic_key_nested' => ['{{ {"foo": { (obj): "foo" }}|join(", ") }}'],
+            'context' => ['{{ _context|join(", ") }}'],
+            'spread_array_operator' => ['{{ [1, 2, ...[5, 6, 7, obj]]|join(",") }}'],
+            'spread_array_operator_var' => ['{{ [1, 2, ...some_array]|join(",") }}'],
         ];
     }