Skip to content

Commit

Permalink
Fix a security issue when an included sandboxed template has been loa…
Browse files Browse the repository at this point in the history
…ded before without the sandbox context
  • Loading branch information
fabpot committed Sep 9, 2024
1 parent 540b54e commit 11f68e2
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 7 deletions.
11 changes: 4 additions & 7 deletions src/Extension/CoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -1431,13 +1431,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 {
Expand All @@ -1452,6 +1445,10 @@ public static function include(Environment $env, $context, $template, $variables
return '';
}

if ($isSandboxed) {
$loaded->unwrap()->checkSecurity();
}

return $loaded->render($variables);
} finally {
if ($isSandboxed && !$alreadySandboxed) {
Expand Down
39 changes: 39 additions & 0 deletions tests/Extension/CoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -354,6 +359,40 @@ public static 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
Expand Down

0 comments on commit 11f68e2

Please sign in to comment.