diff --git a/CHANGELOG b/CHANGELOG index a3b240fff70..5e8a4aa1429 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,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 05098f0b703..9f55e703c04 100644 --- a/src/Environment.php +++ b/src/Environment.php @@ -359,6 +359,8 @@ public function display($name, array $context = []): void public function load($name): TemplateWrapper { if ($name instanceof TemplateWrapper) { + $name->unwrap($this); + return $name; } if ($name instanceof Template) { @@ -503,7 +505,7 @@ public function resolveTemplate($names): TemplateWrapper 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 aeca1467782..f7a58850a67 100644 --- a/src/Template.php +++ b/src/Template.php @@ -73,11 +73,15 @@ abstract public function getSourceContext(): Source; * 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; } @@ -93,7 +97,10 @@ public function getParent(array $context): self|TemplateWrapper|false 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; } @@ -283,11 +290,11 @@ protected function load(string|TemplateWrapper|array $template, int $line, ?int { 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 afadc2b539c..a82b21059f2 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 @@ public function getTemplateName(): string /** * @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 3b37daf91ec..f9e3f887e4c 100644 --- a/tests/CallMacroTest.php +++ b/tests/CallMacroTest.php @@ -235,6 +235,6 @@ private function load(array $templates): Template { $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 a4b0bf7714b..d311c2677b1 100644 --- a/tests/EnvironmentTest.php +++ b/tests/EnvironmentTest.php @@ -85,6 +85,17 @@ public function testAutoescapeOption(): void $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 220b072a8bb..19ee8817d80 100644 --- a/tests/Sandbox/SandboxTest.php +++ b/tests/Sandbox/SandboxTest.php @@ -11,8 +11,10 @@ namespace Twig\Tests\Sandbox; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Twig\Environment; +use Twig\Error\RuntimeError; use Twig\Extension\SandboxExtension; use Twig\Loader\ArrayLoader; use Twig\Markup; @@ -228,6 +230,34 @@ public function testIncludedTemplatesAreSandboxed(): void $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 475c22e6850..87513ed38a0 100644 --- a/tests/TemplateWrapperTest.php +++ b/tests/TemplateWrapperTest.php @@ -22,10 +22,24 @@ use PHPUnit\Framework\TestCase; use Twig\Environment; +use Twig\Error\RuntimeError; use Twig\Loader\ArrayLoader; +use Twig\Template; 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([