From 41103dcdc2daab4c83cdd05b5b4fde5b7e41e635 Mon Sep 17 00:00:00 2001
From: Fabien Potencier <fabien@potencier.org>
Date: Mon, 9 Sep 2024 10:51:06 +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    | 39 +++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php
index 5ac80884a58..4b014b8df93 100644
--- a/src/Extension/CoreExtension.php
+++ b/src/Extension/CoreExtension.php
@@ -1400,13 +1400,6 @@ public static function include(Environment $env, $context, $template, $variables
             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 {
@@ -1419,6 +1412,10 @@ public static function include(Environment $env, $context, $template, $variables
                 }
             }
 
+            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 31458628115..5b8268492a2 100644
--- a/tests/Extension/CoreTest.php
+++ b/tests/Extension/CoreTest.php
@@ -12,8 +12,13 @@
  */
 
 use PHPUnit\Framework\TestCase;
+use Twig\Environment;
 use Twig\Error\RuntimeError;
 use Twig\Extension\CoreExtension;
+use Twig\Extension\SandboxExtension;
+use Twig\Loader\ArrayLoader;
+use Twig\Sandbox\SecurityError;
+use Twig\Sandbox\SecurityPolicy;
 
 class CoreTest extends TestCase
 {
@@ -313,6 +318,40 @@ public function provideCompareCases()
             [1, 42, "\x00\x34\x32"],
         ];
     }
+
+    public function testSandboxedInclude()
+    {
+        $twig = new Environment(new ArrayLoader([
+            'index' => '{{ include("included", sandboxed=true) }}',
+            'included' => '{{ "included"|e }}',
+        ]));
+        $policy = new SecurityPolicy(allowedFunctions: ['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(allowedFunctions: ['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