Fix sandbox handling for __toString()

twigphp · Nov 6, 2024 · cafc608 · cafc608
1 parent ff063af
commit cafc608
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 3 deletions.
diff --git 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) && \is_object($obj) && method_exists($obj, '__toString')) {
             try {
                 $this->policy->checkMethodAllowed($obj, '__toString');

diff --git 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
@@ -38,6 +38,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 = [
@@ -184,10 +185,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');
@@ -210,6 +211,16 @@ public 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(",") }}'],
         ];
     }