Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# 3.29.0 (2026-XX-XX)

* Fix the sandbox resolving `use` trait templates before checking that the `use` tag is allowed
* Fix `html_attr` dropping `style` declarations whose value is `0`, `0.0` or `'0'`
* Fix the `default` filter fallback emitting an undefined variable warning when it uses the null-safe operator
* Fix the `matches` operator silently treating PCRE execution errors as non-matches
Expand Down
5 changes: 4 additions & 1 deletion src/Node/CheckSecurityCallNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
use Twig\Compiler;

/**
* Wires the security checker at the very top of the template constructor, as
* the constructor resolves `use` traits before the sandbox could check them.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
#[YieldReady]
Expand All @@ -26,7 +29,7 @@ class CheckSecurityCallNode extends Node
public function compile(Compiler $compiler)
{
$compiler
->write("\$this->sandbox = \$this->extensions[SandboxExtension::class]->getChecker();\n")
->write("\$this->sandbox = \$env->getExtension(SandboxExtension::class)->getChecker();\n")
;
}
}
26 changes: 26 additions & 0 deletions src/Node/CheckSecurityNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,32 @@ public function __construct(array $usedFilters, array $usedTags, array $usedFunc

public function compile(Compiler $compiler): void
{
if (isset($this->usedTags['use'])) {
$compiler
->write("\n")
->write("protected function checkTraitsAllowed(): void\n")
->write("{\n")
->indent()
->write("if (!\$this->sandbox->isSandboxed(\$this->source)) {\n")
->indent()
->write("return;\n")
->outdent()
->write("}\n\n")
->write("try {\n")
->indent()
->write("\$this->sandbox->checkSecurity(['use'], [], [], [], \$this->source);\n")
->outdent()
->write("} catch (SecurityNotAllowedTagError \$e) {\n")
->indent()
->write('$e->setTemplateLine(')->repr($this->usedTags['use'])->raw(");\n\n")
->write("throw \$e;\n")
->outdent()
->write("}\n")
->outdent()
->write("}\n")
;
}

$compiler
->write("\n")
->write("public function ensureSecurityChecked(): void\n")
Expand Down
3 changes: 2 additions & 1 deletion src/Node/ModuleNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ protected function compileConstructor(Compiler $compiler): void

$countTraits = \count($this->getNode('traits'));
if ($countTraits) {
// traits
$compiler->write("\$this->ensureTraitsAllowed();\n\n");

foreach ($this->getNode('traits') as $i => $trait) {
$node = $trait->getNode('template');

Expand Down
2 changes: 1 addition & 1 deletion src/NodeVisitor/SandboxNodeVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function leaveNode(Node $node, Environment $env): ?Node
if ($node instanceof ModuleNode) {
$this->inAModule = false;

$node->setNode('constructor_end', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_end')]));
$node->setNode('constructor_start', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_start')]));
$node->setNode('class_end', new Nodes([new CheckSecurityNode($this->filters, $this->tags, $this->functions, $this->tests), $node->getNode('class_end')]));
}

Expand Down
25 changes: 25 additions & 0 deletions src/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,31 @@ public function ensureSecurityChecked(): void
{
}

/**
* Checks the "use" tag against the sandbox policy.
*
* The constructor resolves "use" traits eagerly, which reaches the loader,
* so that tag alone is checked here; the rest of the policy still runs at
* render time.
*
* @internal
*/
public function ensureTraitsAllowed(): void
{
try {
$this->checkTraitsAllowed();
} catch (\Throwable $e) {
$this->handleException($e);
}
}

/**
* @internal
*/
protected function checkTraitsAllowed(): void
{
}

/**
* Auto-generated method to display the template with the given context.
*
Expand Down
3 changes: 3 additions & 0 deletions tests/Extension/SandboxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ public static function getStrictSandboxRejectsGrandfatheredTagsTests()
{
yield ['extends', '{% extends "1_empty" %}'];
yield ['use', '{% use "1_empty" %}'];
yield 'use of a missing template is rejected before the loader is reached' => ['use', '{% use "does_not_exist" %}'];
yield 'use of a missing block is rejected before the trait is resolved' => ['use', '{% use "1_layout" with does_not_exist as alias %}'];
yield 'use of a non-traitable template is rejected before the trait is resolved' => ['use', '{% use "1_child" %}'];
}

/**
Expand Down
78 changes: 78 additions & 0 deletions tests/Sandbox/SandboxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Twig\Environment;
use Twig\Error\RuntimeError;
use Twig\Extension\SandboxExtension;
use Twig\Loader\ArrayLoader;
use Twig\Markup;
Expand Down Expand Up @@ -244,6 +245,65 @@ public function testTheExtendsTagMustBeAllowed(): void
$denying->render('index');
}

public function testTheUseTagMustBeAllowed(): void
{
$templates = [
'index' => '{% use "blocks" with content as base_content %}{{ block("base_content") }}',
'blocks' => '{% block content %}trait content{% endblock %}',
];

$allowing = new Sandbox(self::env($templates), self::strictPolicy(tags: ['use', 'block'], functions: ['block']));
$this->assertSame('trait content', $allowing->render('index'));

$denying = new Sandbox(self::env(['index' => '{% use "missing" with content as base_content %}']), self::strictPolicy());
$this->expectException(SecurityNotAllowedTagError::class);
$this->expectExceptionMessage('Tag "use" is not allowed');
$denying->render('index');
}

public function testTheUseTagIsCheckedWithoutCheckingTheRestOfThePolicy(): void
{
// "middle" is only reachable as a trait, and the block carrying the
// forbidden filter is overridden by "index", so it never renders.
$sandbox = new Sandbox(self::env([
'index' => '{% use "middle" %}{% block content %}SAFE{% endblock %}',
'middle' => '{% use "leaf" %}{% block content %}{{ "bad"|upper }}{% endblock %}',
'leaf' => '',
]), self::strictPolicy(tags: ['use', 'block']));

$this->assertSame('SAFE', $sandbox->render('index'));
}

public function testTheUseTagIsCheckedBeforeTheTraitTemplateIsLoaded(): void
{
$sandbox = new Sandbox(self::env([
'index' => '{{ block("b", "receiver") is defined ? "YES" : "NO" }}',
'receiver' => '{% use "missing" %}',
]), self::strictPolicy(tags: ['block'], functions: ['block']));

$this->expectException(SecurityNotAllowedTagError::class);
$this->expectExceptionMessage('Tag "use" is not allowed');
$sandbox->render('index');
}

public function testAPolicyFailureWhileResolvingTraitsKeepsItsTwigContext(): void
{
$sandbox = new Sandbox(self::env([
'index' => "{% use \"empty\" %}\n{{ 'a'|upper }}",
'empty' => '',
]), new ThrowingOnUseSecurityPolicy());

try {
$sandbox->render('index');
$this->fail('The policy failure should have been reported.');
} catch (RuntimeError $e) {
$this->assertStringContainsString('Policy backend unreachable', $e->getMessage());
$this->assertSame('index', $e->getSourceContext()?->getName());
$this->assertSame(2, $e->getTemplateLine());
$this->assertInstanceOf(\RuntimeException::class, $e->getPrevious());
}
}

public function testARenderOnAnotherEnvironmentDuringASandboxedRenderIsNotSandboxed(): void
{
$app = new Environment(new ArrayLoader(['trusted' => '{{ value|upper }}']), ['autoescape' => false]);
Expand Down Expand Up @@ -377,6 +437,24 @@ private static function strictPolicy(array $tags = [], array $filters = [], arra
}
}

final class ThrowingOnUseSecurityPolicy implements SecurityPolicyInterface
{
public function checkSecurity($tags, $filters, $functions, $tests = []): void
{
if (\in_array('use', $tags, true)) {
throw new \RuntimeException('Policy backend unreachable.');
}
}

public function checkMethodAllowed($obj, $method): void
{
}

public function checkPropertyAllowed($obj, $property): void
{
}
}

final class SandboxTestObject implements \Stringable
{
public $name = 'fabien';
Expand Down
Loading