bug #4918 Check that the use tag is allowed before resolving trait templates (fabpot)

This PR was merged into the 3.x branch.

Discussion
----------

Check that the use tag is allowed before resolving trait templates

Commits
-------

18863f0371 Check that the use tag is allowed before resolving trait templates

CHANGELOG

@@ -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

src/Node/CheckSecurityCallNode.php

@@ -15,6 +15,9 @@ use Twig\Attribute\YieldReady;
 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]
@@ -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")
         ;
     }
 }

src/Node/CheckSecurityNode.php

@@ -47,6 +47,32 @@ class CheckSecurityNode extends Node
 
     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")

src/Node/ModuleNode.php

@@ -218,7 +218,8 @@ final class ModuleNode extends Node implements CoercesChildrenToStringInterface
 
         $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');
 

src/NodeVisitor/SandboxNodeVisitor.php

@@ -125,7 +125,7 @@ final class SandboxNodeVisitor implements NodeVisitorInterface
         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')]));
         }
 

src/Template.php

@@ -508,6 +508,31 @@ abstract class Template
     {
     }
 
+    /**
+     * 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.
      *

tests/Extension/SandboxTest.php

@@ -293,6 +293,9 @@ class SandboxTest extends TestCase
     {
         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" %}'];
     }
 
     /**

tests/Sandbox/SandboxTest.php

@@ -13,6 +13,7 @@ namespace Twig\Tests\Sandbox;
 
 use PHPUnit\Framework\TestCase;
 use Twig\Environment;
+use Twig\Error\RuntimeError;
 use Twig\Extension\SandboxExtension;
 use Twig\Loader\ArrayLoader;
 use Twig\Markup;
@@ -244,6 +245,65 @@ class SandboxTest extends TestCase
         $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]);
@@ -377,6 +437,24 @@ class SandboxTest extends TestCase
     }
 }
 
+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';