Fix a security issue when an included sandboxed template has been loaded before without the sandbox context

Author: fabpot

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) {

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