From 2102dd135986db79192d26fb5f5817a566e0a7de Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 9 Sep 2024 18:53:26 +0200 Subject: [PATCH] Fix a security issue when an included sandboxed template has been loaded before without the sandbox context --- src/Extension/CoreExtension.php | 11 ++++------ tests/Extension/CoreTest.php | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php index 5c4087ec217..18fc72b5847 100644 --- a/src/Extension/CoreExtension.php +++ b/src/Extension/CoreExtension.php @@ -1268,13 +1268,6 @@ function twig_include(Environment $env, $context, $template, $variables = [], $w if (!$alreadySandboxed = $sandbox->isSandboxed()) { $sandbox->enableSandbox(); } - - foreach ((\is_array($template) ? $template : [$template]) as $name) { - // if a Template instance is passed, it might have been instantiated outside of a sandbox, check security - if ($name instanceof TemplateWrapper || $name instanceof Template) { - $name->unwrap()->checkSecurity(); - } - } } try { @@ -1287,6 +1280,10 @@ function twig_include(Environment $env, $context, $template, $variables = [], $w } } + if ($isSandboxed && $loaded) { + $loaded->unwrap()->checkSecurity(); + } + return $loaded ? $loaded->render($variables) : ''; } finally { if ($isSandboxed && !$alreadySandboxed) { diff --git a/tests/Extension/CoreTest.php b/tests/Extension/CoreTest.php index 5f82ccad58f..0e73b8c1363 100644 --- a/tests/Extension/CoreTest.php +++ b/tests/Extension/CoreTest.php @@ -14,7 +14,11 @@ use PHPUnit\Framework\TestCase; use Twig\Environment; use Twig\Error\RuntimeError; +use Twig\Extension\SandboxExtension; +use Twig\Loader\ArrayLoader; use Twig\Loader\LoaderInterface; +use Twig\Sandbox\SecurityError; +use Twig\Sandbox\SecurityPolicy; class CoreTest extends TestCase { @@ -251,6 +255,40 @@ public function provideSliceFilterCases() [[], new \ArrayIterator([1, 2]), 3], ]; } + + public function testSandboxedInclude() + { + $twig = new Environment(new ArrayLoader([ + 'index' => '{{ include("included", sandboxed=true) }}', + 'included' => '{{ "included"|e }}', + ])); + $policy = new SecurityPolicy([], [], [], [], ['include']); + $sandbox = new SandboxExtension($policy, false); + $twig->addExtension($sandbox); + + // We expect a compile error + $this->expectException(SecurityError::class); + $twig->render('index'); + } + + public function testSandboxedIncludeWithPreloadedTemplate() + { + $twig = new Environment(new ArrayLoader([ + 'index' => '{{ include("included", sandboxed=true) }}', + 'included' => '{{ "included"|e }}', + ])); + $policy = new SecurityPolicy([], [], [], [], ['include']); + $sandbox = new SandboxExtension($policy, false); + $twig->addExtension($sandbox); + + // The template is loaded without the sandbox enabled + // so, no compile error + $twig->load('included'); + + // We expect a runtime error + $this->expectException(SecurityError::class); + $twig->render('index'); + } } final class CoreTestIteratorAggregate implements \IteratorAggregate