mirror of
https://github.com/twigphp/Twig.git
synced 2026-08-23 04:32:30 +00:00
Fix sandbox __toString bypass via Stringable + Traversable containers
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user