diff --git a/CHANGELOG b/CHANGELOG index d5222608b..ddff4d367 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,7 +7,7 @@ * Escape root profile name in `HtmlDumper` * Deprecate the `Twig\Sandbox\SourcePolicyInterface` interface with no replacement * Fix sandbox bypass in the "column" filter when sandboxing is enabled via `SourcePolicyInterface` - * Fix sandbox `__toString` bypass via `Traversable` arguments to the `join` and `replace` filters + * Fix sandbox `__toString` bypass via `Traversable` arguments to the `join` and `replace` filters (also covers containers that implement both `Stringable` and `Traversable`) * Fix sandbox `__toString` bypass via the `in` and `not in` operators # 3.26.0 (2026-05-20) diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php index b105375e4..947f9f87e 100644 --- a/src/Extension/SandboxExtension.php +++ b/src/Extension/SandboxExtension.php @@ -132,17 +132,11 @@ final class SandboxExtension extends AbstractExtension return $obj; } - // A non-Stringable Traversable would later be materialised (e.g. by filters such as - // `join` or `replace`) and its elements coerced to string by PHP itself, bypassing - // the policy. Materialise it now and recursively check the contents. - if ($obj instanceof \Traversable && !$obj instanceof \Stringable && $this->isSandboxed($source)) { - $obj = iterator_to_array($obj); - $this->ensureToStringAllowedForArray($obj, $lineno, $source); - + if (!$this->isSandboxed($source)) { return $obj; } - if ($obj instanceof \Stringable && $this->isSandboxed($source)) { + if ($obj instanceof \Stringable) { try { $this->policy->checkMethodAllowed($obj, '__toString'); } catch (SecurityNotAllowedMethodError $e) { @@ -153,6 +147,25 @@ final class SandboxExtension extends AbstractExtension } } + // A Traversable would later be materialised (e.g. by filters such as `join` + // or `replace`) and its elements coerced to string by PHP itself, bypassing + // the policy. Materialise it now and recursively check the contents. This + // also applies to objects that implement both `Stringable` and `Traversable`: + // the `__toString` check above only validates the container's own coercion, + // not the elements yielded by `getIterator()`. + if ($obj instanceof \Traversable) { + $array = iterator_to_array($obj); + $this->ensureToStringAllowedForArray($array, $lineno, $source); + + // Return the materialised array only when the object is not also + // Stringable, so that callers that rely on `__toString` (e.g. `{{ obj }}`) + // keep working. Plain consumers of iterables (join, replace, ...) call + // `iterator_to_array()` again, so the extra materialisation is benign. + if (!$obj instanceof \Stringable) { + return $array; + } + } + return $obj; } diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index 9fcfb4a70..a6d13ed1f 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -64,6 +64,8 @@ class SandboxTest extends TestCase 'iterator' => new \ArrayIterator(['a', new FooObject()]), 'iterator_map' => new \ArrayIterator(['__toString' => new FooObject()]), 'iterator_nested' => new \ArrayIterator(['a', new \ArrayIterator(['b', new FooObject()])]), + 'stringable_iterator' => new StringableTraversableObject(['a', new FooObject()]), + 'stringable_iterator_map' => new StringableTraversableObject(['__toString' => new FooObject()]), ]; self::$params['recursion'][] = &self::$params['recursion']; self::$params['recursion'][] = new FooObject(); @@ -1176,6 +1178,88 @@ EOF } } + /** + * @dataProvider getStringableTraversableBypassTemplates + */ + public function testSandboxBlocksToStringInStringableTraversable(string $template) + { + $twig = $this->getEnvironment( + true, + [], + ['index' => $template], + [], + ['join', 'replace'], + ['Twig\Tests\Extension\StringableTraversableObject' => ['__tostring']], + ); + + try { + $twig->load('index')->render(self::$params); + $this->fail('Sandbox should block __toString on objects yielded by a Stringable+Traversable container, even when the container\'s own __toString is allowed.'); + } catch (SecurityNotAllowedMethodError $e) { + $this->assertSame('Twig\Tests\Extension\FooObject', $e->getClassName()); + $this->assertSame('__tostring', $e->getMethodName()); + } + } + + public static function getStringableTraversableBypassTemplates(): iterable + { + yield 'join' => ['{{ stringable_iterator|join(", ") }}']; + yield 'replace' => ['{{ "__toString"|replace(stringable_iterator_map) }}']; + } + + /** + * @dataProvider getStringableTraversableBypassTemplates + */ + public function testSourcePolicySandboxBlocksToStringInStringableTraversable(string $template) + { + $sourcePolicy = new class implements SourcePolicyInterface { + public function enableSandbox(Source $source): bool + { + return true; + } + }; + + $twig = $this->getEnvironment( + false, + [], + ['index' => $template], + [], + ['join', 'replace'], + ['Twig\Tests\Extension\StringableTraversableObject' => ['__tostring']], + [], + [], + $sourcePolicy, + ); + + try { + $twig->load('index')->render(self::$params); + $this->fail('Sandbox should block __toString on objects yielded by a Stringable+Traversable container under a SourcePolicyInterface-only sandbox.'); + } catch (SecurityNotAllowedMethodError $e) { + $this->assertSame('Twig\Tests\Extension\FooObject', $e->getClassName()); + $this->assertSame('__tostring', $e->getMethodName()); + } + } + + public function testSandboxAllowsPrintingStringableTraversableWhenToStringAllowed() + { + // Printing the container itself yields its `__toString()` value. The + // sandbox materialises the iterable to also policy-check the elements + // (some consumers like `join`/`replace` would coerce them too), so the + // inner items must not contain anything that violates the policy. + $twig = $this->getEnvironment( + true, + ['autoescape' => 'html'], + ['index' => '{{ obj }}'], + [], + ['escape'], + ['Twig\Tests\Extension\StringableTraversableObject' => ['__tostring']], + ); + + $params = ['obj' => new StringableTraversableObject(['a', 'b'])]; + + $this->assertSame('stringable-traversable', $twig->load('index')->render($params)); + } + public function testSourcePolicySandboxBlocksToStringInTraversableJoin() { $sourcePolicy = new class implements SourcePolicyInterface { @@ -1722,3 +1806,24 @@ class ColumnObject { public $bar = 'bar'; } + +// Implements both Stringable and Traversable: a sandbox policy may legitimately +// allow the container's own `__toString`, but the elements yielded by +// `getIterator()` must still be policy-checked when consumers (`join`, `replace`, +// ...) materialise the iterable and coerce its contents to string. +class StringableTraversableObject implements \IteratorAggregate, \Stringable +{ + public function __construct(private array $items) + { + } + + public function __toString(): string + { + return 'stringable-traversable'; + } + + public function getIterator(): \Traversable + { + yield from $this->items; + } +}