From b1cac7c3abcc80c7f8773541657f6f13053a311c Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 7 Jun 2026 09:46:35 +0200 Subject: [PATCH] Enforce the sandbox Markup exception for every security policy --- CHANGELOG | 1 + src/Extension/SandboxExtension.php | 5 ++- src/Markup.php | 13 +++----- src/Sandbox/SecurityPolicy.php | 7 ---- tests/Extension/SandboxTest.php | 53 ++++++++++++++++++++++++++++++ 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a7652bc07bc..59f59f4e08d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ # 4.0.0 (2026-XX-XX) * Add the `isAlwaysAllowedInSandbox()` method to `Twig\TwigCallableInterface` and `Twig\TokenParser\TokenParserInterface` + * Always allow printing a `Markup` object in a sandbox, whatever the security policy is * Remove the `Twig\Sandbox\SourcePolicyInterface` interface and the corresponding argument of `Twig\Extension\SandboxExtension::__construct()` * Enforce the `parent`, `block`, and `attribute` functions against the sandbox `allowedFunctions` allow-list diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php index b11f34c2acb..efd3e26a750 100644 --- a/src/Extension/SandboxExtension.php +++ b/src/Extension/SandboxExtension.php @@ -11,6 +11,7 @@ namespace Twig\Extension; +use Twig\Markup; use Twig\NodeVisitor\SandboxNodeVisitor; use Twig\Sandbox\SecurityNotAllowedMethodError; use Twig\Sandbox\SecurityNotAllowedPropertyError; @@ -139,7 +140,9 @@ private function doEnsureToStringAllowed($obj, int $lineno, ?Source $source, \Sp return $obj; } - if ($obj instanceof \Stringable) { + // Markup carries content that Twig already considers safe, so its + // __toString() is always allowed, whatever the security policy is. + if ($obj instanceof \Stringable && !$obj instanceof Markup) { try { $this->policy->checkMethodAllowed($obj, '__toString'); } catch (SecurityNotAllowedMethodError $e) { diff --git a/src/Markup.php b/src/Markup.php index def6603248c..e9f737dcdd1 100644 --- a/src/Markup.php +++ b/src/Markup.php @@ -14,14 +14,11 @@ /** * Marks a content as safe. * - * Instances of this class (and any subclass) are trusted by the Twig - * sandbox: method calls and property accesses on a Markup instance bypass - * the SecurityPolicy method/property allowlists. This is by design: Markup - * represents content that has already been deemed safe to output. - * - * As a consequence, when extending this class, you are responsible for - * ensuring that every method and property exposed by your subclass is - * safe to call from a sandboxed template. + * Instances of this class are trusted by the Twig sandbox when output as + * strings: their __toString() method is always allowed. This is by design as + * Markup represents content that has already been deemed safe to output. + * Regular method calls and property accesses are still controlled by the + * SecurityPolicy method/property allowlists. * * @author Fabien Potencier */ diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php index 4f68acb3be3..025e82b6b21 100644 --- a/src/Sandbox/SecurityPolicy.php +++ b/src/Sandbox/SecurityPolicy.php @@ -11,9 +11,6 @@ namespace Twig\Sandbox; -use Twig\Markup; -use Twig\Template; - /** * Represents a security policy which need to be enforced when sandbox mode is enabled. * @@ -115,10 +112,6 @@ public function checkSecurity($tags, $filters, $functions): void public function checkMethodAllowed($obj, $method): void { - if ($obj instanceof Template || $obj instanceof Markup) { - return; - } - $allowed = false; $method = strtolower($method); foreach ($this->allowedMethods as $class => $methods) { diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index 60a8dc44ad3..bec619ac721 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -28,6 +28,7 @@ use Twig\Extension\SandboxExtension; use Twig\Extension\StringLoaderExtension; use Twig\Loader\ArrayLoader; +use Twig\Markup; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Node; use Twig\Node\Nodes; @@ -40,9 +41,11 @@ use Twig\Sandbox\SecurityNotAllowedPropertyError; use Twig\Sandbox\SecurityNotAllowedTagError; use Twig\Sandbox\SecurityPolicy; +use Twig\Sandbox\SecurityPolicyInterface; use Twig\Source; use Twig\Token; use Twig\TokenParser\AbstractTokenParser; +use Twig\TokenParser\TokenParserInterface; use Twig\TwigFilter; use Twig\TwigFunction; use Twig\TwigTest; @@ -661,6 +664,39 @@ public function testSandboxAllowMethodToStringDisabled() $this->assertEquals(1, FooObject::$called['__toString'], 'Sandbox only calls method once'); } + public function testSandboxAllowsPrintingMarkup() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ markup }}']); + + $this->assertSame('safe', $twig->load('index')->render(['markup' => new Markup('safe', 'UTF-8')])); + } + + public function testSandboxAllowsPrintingMarkupWithACustomPolicyThatAllowsNothing() + { + $loader = new ArrayLoader(['index' => '{{ markup }}']); + $twig = new Environment($loader, ['cache' => false, 'autoescape' => false]); + $twig->addExtension(new SandboxExtension(new DenyEverythingSecurityPolicy(), true)); + + $this->assertSame('safe', $twig->load('index')->render(['markup' => new Markup('safe', 'UTF-8')])); + } + + public function testSandboxAppliesThePolicyToTemplateMethods() + { + $template = $this->getEnvironment(true, [], ['index' => 'foo'])->load('index')->unwrap(); + $policy = new SecurityPolicy(); + + $this->expectException(SecurityNotAllowedMethodError::class); + $policy->checkMethodAllowed($template, 'getTemplateName'); + } + + public function testSandboxAppliesThePolicyToMarkupMethods() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ markup.getCharset() }}']); + + $this->expectException(SecurityNotAllowedMethodError::class); + $twig->load('index')->render(['markup' => new Markup('safe', 'UTF-8')]); + } + public function testSandboxUnallowedFunction() { $twig = $this->getEnvironment(true, [], self::$templates); @@ -1730,3 +1766,20 @@ public function getTag(): string return 'gated_tag'; } } + +class DenyEverythingSecurityPolicy implements SecurityPolicyInterface +{ + public function checkSecurity($tags, $filters, $functions): void + { + } + + public function checkMethodAllowed($obj, $method): void + { + throw new SecurityNotAllowedMethodError(\sprintf('Calling "%s" method on a "%s" object is not allowed.', $method, $obj::class), $obj::class, $method); + } + + public function checkPropertyAllowed($obj, $property): void + { + throw new SecurityNotAllowedPropertyError(\sprintf('Calling "%s" property on a "%s" object is not allowed.', $property, $obj::class), $obj::class, $property); + } +}