Skip to content

Commit

Permalink
Fix sandbox handling for __toString()
Browse files Browse the repository at this point in the history
  • Loading branch information
fabpot authored and nicolas-grekas committed Nov 5, 2024
1 parent 126b2c9 commit 2bb8c24
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
8 changes: 8 additions & 0 deletions src/Extension/SandboxExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
15 changes: 14 additions & 1 deletion src/NodeVisitor/SandboxNodeVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
15 changes: 13 additions & 2 deletions tests/Extension/SandboxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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');
Expand All @@ -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(",") }}'],
];
}

Expand Down

0 comments on commit 2bb8c24

Please sign in to comment.