From c1fc112047bd2c8bf5c4f4d762e52e7f48ccc602 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 28 Aug 2026 10:10:55 +0200 Subject: [PATCH 1/2] Reject cross-environment template wrappers --- CHANGELOG | 1 + src/Environment.php | 4 +++- src/Template.php | 17 ++++++++++++----- src/TemplateWrapper.php | 10 +++++++--- tests/CallMacroTest.php | 2 +- tests/EnvironmentTest.php | 11 +++++++++++ tests/Sandbox/SandboxTest.php | 29 +++++++++++++++++++++++++++++ tests/TemplateWrapperTest.php | 13 +++++++++++++ 8 files changed, 77 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b0fb15187..36b8b4247 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ * Add `TempestMarkdown` to use `tempest/markdown` as the `markdown_to_html` converter * Add the `include_only` function to render a template without giving it access to the current context * Add the `Twig\Sandbox\SandboxInterface` interface and `Twig\Sandbox\Sandbox` class to render untrusted templates through a dedicated, always-sandboxed environment crafted for it + * Reject `TemplateWrapper` instances created by another `Environment` * Add the `Twig\Extension\SandboxBridgeExtension` to render sandboxed templates from trusted templates with an explicit output escaping strategy * Extract the sandbox runtime enforcement into a new internal `Twig\Sandbox\SecurityChecker` class used by compiled templates and `CoreExtension` * Mark `SandboxExtension` as internal, use `Twig\Sandbox\Sandbox` instead diff --git a/src/Environment.php b/src/Environment.php index 05098f0b7..9f55e703c 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -359,6 +359,8 @@ class Environment public function load($name): TemplateWrapper { if ($name instanceof TemplateWrapper) { + $name->unwrap($this); + return $name; } if ($name instanceof Template) { @@ -503,7 +505,7 @@ class Environment return new TemplateWrapper($this, $name); } if ($name instanceof TemplateWrapper) { - return $name; + return $this->load($name); } if (1 !== $count && !$this->getLoader()->exists($name)) { diff --git a/src/Template.php b/src/Template.php index a3023489d..17842382a 100644 --- a/src/Template.php +++ b/src/Template.php @@ -73,11 +73,15 @@ abstract class Template * This method is for internal use only and should never be called * directly. * - * @return self|TemplateWrapper|false The parent template or false if there is no parent + * @return self|false The parent template or false if there is no parent */ - public function getParent(array $context): self|TemplateWrapper|false + public function getParent(array $context): self|false { if (null !== $this->parent) { + if ($this->parent instanceof TemplateWrapper) { + $this->parent = $this->load($this->parent, -1); + } + return $this->parent; } @@ -99,7 +103,10 @@ abstract class Template return false; } - if ($parent instanceof self || $parent instanceof TemplateWrapper) { + if ($parent instanceof TemplateWrapper) { + $parent = $this->load($parent, -1); + } + if ($parent instanceof self) { return $this->parents[$parent->getSourceContext()->getName()] = $parent; } @@ -298,11 +305,11 @@ abstract class Template { try { if (\is_array($template)) { - return $this->env->resolveTemplate($template)->unwrap(); + return $this->env->resolveTemplate($template)->unwrap($this->env); } if ($template instanceof TemplateWrapper) { - return $template->unwrap(); + return $template->unwrap($this->env); } if ($template === $this->getTemplateName()) { diff --git a/src/TemplateWrapper.php b/src/TemplateWrapper.php index c08fca5e3..185b565e1 100644 --- a/src/TemplateWrapper.php +++ b/src/TemplateWrapper.php @@ -11,6 +11,8 @@ namespace Twig; +use Twig\Error\RuntimeError; + /** * Exposes a template to userland. * @@ -96,11 +98,13 @@ final class TemplateWrapper /** * @internal - * - * @return Template */ - public function unwrap() + public function unwrap(Environment $env): Template { + if ($this->env !== $env) { + throw new RuntimeError(\sprintf('A "%s" can only be used with the "%s" that created it.', self::class, Environment::class)); + } + return $this->template; } } diff --git a/tests/CallMacroTest.php b/tests/CallMacroTest.php index 3b37daf91..f9e3f887e 100644 --- a/tests/CallMacroTest.php +++ b/tests/CallMacroTest.php @@ -235,6 +235,6 @@ class CallMacroTest extends TestCase { $twig = new Environment(new ArrayLoader($templates)); - return $twig->load('index')->unwrap(); + return $twig->load('index')->unwrap($twig); } } diff --git a/tests/EnvironmentTest.php b/tests/EnvironmentTest.php index a4b0bf771..d311c2677 100644 --- a/tests/EnvironmentTest.php +++ b/tests/EnvironmentTest.php @@ -85,6 +85,17 @@ class EnvironmentTest extends TestCase $this->assertEquals('foo\u003Cbr\/\u0020\u003E foo\u003Cbr\/\u0020\u003E', $twig->render('js', ['bar' => 'foo
'])); } + public function testRejectsTemplateWrapperFromAnotherEnvironment(): void + { + $foreign = new Environment(new ArrayLoader(['index' => 'foreign'])); + $twig = new Environment(new ArrayLoader()); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('can only be used with the "Twig\\Environment" that created it'); + + $twig->load($foreign->load('index')); + } + public function escapingStrategyCallback($name) { return $name; diff --git a/tests/Sandbox/SandboxTest.php b/tests/Sandbox/SandboxTest.php index 63923d0de..76ab18f63 100644 --- a/tests/Sandbox/SandboxTest.php +++ b/tests/Sandbox/SandboxTest.php @@ -11,6 +11,7 @@ namespace Twig\Tests\Sandbox; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Twig\Environment; use Twig\Error\RuntimeError; @@ -229,6 +230,34 @@ class SandboxTest extends TestCase $sandbox->render('index'); } + /** + * @dataProvider provideForeignTemplateWrapperUsages + */ + #[DataProvider('provideForeignTemplateWrapperUsages')] + public function testRejectsTemplateWrapperFromAnotherEnvironment(string $template, string $foreignTemplate, array $tags = [], array $functions = []): void + { + $foreign = self::env(['foreign' => $foreignTemplate]); + $sandbox = new Sandbox(self::env(['index' => $template]), self::strictPolicy(tags: $tags, functions: $functions)); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('can only be used with the "Twig\\Environment" that created it'); + + $sandbox->render('index', ['foreign' => $foreign->load('foreign')]); + } + + public static function provideForeignTemplateWrapperUsages(): iterable + { + yield 'include tag' => ['{% include foreign %}', 'foreign content', ['include']]; + yield 'include function' => ['{{ include(foreign) }}', 'foreign content', [], ['include']]; + yield 'include function fallback' => ['{{ include(["missing", foreign]) }}', 'foreign content', [], ['include']]; + yield 'include_only function' => ['{{ include_only(foreign) }}', 'foreign content', [], ['include_only']]; + yield 'extends tag' => ['{% extends foreign %}', 'foreign content', ['extends']]; + yield 'embed tag' => ['{% embed foreign %}{% endembed %}', 'foreign content', ['embed', 'extends']]; + yield 'import tag' => ['{% import foreign as macros %}{{ macros.foo() }}', '{% macro foo() %}foreign content{% endmacro %}', ['import']]; + yield 'from tag' => ['{% from foreign import foo %}{{ foo() }}', '{% macro foo() %}foreign content{% endmacro %}', ['from']]; + yield 'block function' => ['{{ block("content", foreign) }}', '{% block content %}foreign content{% endblock %}', [], ['block']]; + } + public function testTheExtendsTagMustBeAllowed(): void { $templates = [ diff --git a/tests/TemplateWrapperTest.php b/tests/TemplateWrapperTest.php index 197e3aa13..1807db61d 100644 --- a/tests/TemplateWrapperTest.php +++ b/tests/TemplateWrapperTest.php @@ -26,10 +26,23 @@ use Twig\Environment; use Twig\Error\LoaderError; use Twig\Error\RuntimeError; use Twig\Loader\ArrayLoader; +use Twig\Template; use Twig\TwigFunction; class TemplateWrapperTest extends TestCase { + public function testUnwrapChecksTheEnvironment(): void + { + $twig = new Environment(new ArrayLoader(['index' => 'content'])); + $wrapper = $twig->load('index'); + + $this->assertInstanceOf(Template::class, $wrapper->unwrap($twig)); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('can only be used with the "Twig\\Environment" that created it'); + $wrapper->unwrap(new Environment(new ArrayLoader())); + } + public function testHasGetBlocks(): void { $twig = new Environment(new ArrayLoader([ From 83e8f7e123e3f29c6bdf54af2b94e883339717d3 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 12 Sep 2026 09:57:01 +0200 Subject: [PATCH 2/2] Reject cross-environment template wrappers in block chains --- src/BlockChain.php | 9 ++------- src/Template.php | 4 ---- src/TemplateWrapper.php | 10 +++++++++- tests/BlockChainTest.php | 6 +++--- tests/TemplateTest.php | 8 ++++---- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/BlockChain.php b/src/BlockChain.php index e68f145da..7b21c595f 100644 --- a/src/BlockChain.php +++ b/src/BlockChain.php @@ -49,12 +49,11 @@ final class BlockChain throw new \TypeError(\sprintf('Block chain templates must be strings or "%s" instances, "%s" given.', TemplateWrapper::class, get_debug_type($template))); } - $template = $template->unwrap(); if (!$template->isOwnedBy($env)) { throw new \LogicException('A block chain cannot contain templates from different Twig environments.'); } - $this->templates[] = $template; + $this->templates[] = $template->unwrap($env); } if (!$this->templates) { @@ -173,11 +172,7 @@ final class BlockChain $parent = $template->getParent($context); $fixed = $fixed && $template->hasFixedParent(); - // a dynamic parent expression can evaluate to a template from another environment - $template = $parent instanceof TemplateWrapper ? $parent->unwrap() : $parent; - if (false !== $template && !$template->isOwnedBy($this->env)) { - throw new \LogicException('A block chain cannot contain templates from different Twig environments.'); - } + $template = $parent; } while (false !== $template); } diff --git a/src/Template.php b/src/Template.php index 17842382a..c2c1d37a4 100644 --- a/src/Template.php +++ b/src/Template.php @@ -78,10 +78,6 @@ abstract class Template public function getParent(array $context): self|false { if (null !== $this->parent) { - if ($this->parent instanceof TemplateWrapper) { - $this->parent = $this->load($this->parent, -1); - } - return $this->parent; } diff --git a/src/TemplateWrapper.php b/src/TemplateWrapper.php index 185b565e1..ab1c414fd 100644 --- a/src/TemplateWrapper.php +++ b/src/TemplateWrapper.php @@ -96,12 +96,20 @@ final class TemplateWrapper return $this->template->getTemplateName(); } + /** + * @internal + */ + public function isOwnedBy(Environment $env): bool + { + return $this->env === $env && $this->template->isOwnedBy($env); + } + /** * @internal */ public function unwrap(Environment $env): Template { - if ($this->env !== $env) { + if (!$this->isOwnedBy($env)) { throw new RuntimeError(\sprintf('A "%s" can only be used with the "%s" that created it.', self::class, Environment::class)); } diff --git a/tests/BlockChainTest.php b/tests/BlockChainTest.php index f096575fa..85e230d65 100644 --- a/tests/BlockChainTest.php +++ b/tests/BlockChainTest.php @@ -253,7 +253,7 @@ class BlockChainTest extends TestCase { $twig = new Environment(new ArrayLoader(['theme' => ''])); $other = new Environment(new ArrayLoader(['theme' => ''])); - $wrapper = new TemplateWrapper($twig, $other->load('theme')->unwrap()); + $wrapper = new TemplateWrapper($twig, $other->load('theme')->unwrap($other)); $this->expectException(\LogicException::class); $this->expectExceptionMessage('A block chain cannot contain templates from different Twig environments.'); @@ -267,8 +267,8 @@ class BlockChainTest extends TestCase $other = new Environment(new ArrayLoader(['parent' => ''])); $chain = new BlockChain($twig, ['theme'], ['parent' => $other->load('parent')]); - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('A block chain cannot contain templates from different Twig environments.'); + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('A "Twig\TemplateWrapper" can only be used with the "Twig\Environment" that created it in "theme" at line 1.'); $chain->getBlockNames(); } diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php index 86997d2db..8db4b2113 100644 --- a/tests/TemplateTest.php +++ b/tests/TemplateTest.php @@ -236,7 +236,7 @@ class TemplateTest extends TestCase 'parent' => '{% block content %}{{ missing.value }}{% endblock %}', 'child' => '{% extends "parent" %}', ]), ['debug' => $debug, 'strict_variables' => true, 'use_yield' => false]); - $template = $twig->load('child')->unwrap(); + $template = $twig->load('child')->unwrap($twig); $level = ob_get_level(); try { @@ -261,14 +261,14 @@ class TemplateTest extends TestCase 'dynamic_parent' => '{% extends parent %}', ])); - $this->assertTrue($twig->load('no_parent')->unwrap()->hasFixedParent()); + $this->assertTrue($twig->load('no_parent')->unwrap($twig)->hasFixedParent()); - $constant = $twig->load('constant_parent')->unwrap(); + $constant = $twig->load('constant_parent')->unwrap($twig); $this->assertFalse($constant->hasFixedParent()); $constant->getParent([]); $this->assertTrue($constant->hasFixedParent()); - $dynamic = $twig->load('dynamic_parent')->unwrap(); + $dynamic = $twig->load('dynamic_parent')->unwrap($twig); $this->assertFalse($dynamic->hasFixedParent()); $dynamic->getParent(['parent' => 'no_parent']); $this->assertFalse($dynamic->hasFixedParent());