diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 204649b0e..bc79c989d 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -17,7 +17,7 @@ return (new Config()) 'single_line_throw' => false, 'ordered_imports' => true, 'phpdoc_types_order' => ['null_adjustment' => 'always_last', 'sort_algorithm' => 'none'], - 'no_superfluous_phpdoc_tags' => ['allow_mixed' => true], + 'no_superfluous_phpdoc_tags' => ['allow_mixed' => true, 'allow_unused_params' => true], ]) ->setRuleCustomisationPolicy(new class implements PhpCsFixer\Config\RuleCustomisationPolicyInterface { public function getPolicyVersionForCache(): string diff --git a/CHANGELOG b/CHANGELOG index e89fde984..ed3a0263f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,7 +9,8 @@ * Make the `include()` function return a `Markup` object so an assigned result is not re-escaped when printed * Skip the sandbox `__toString` check on arguments whose PHP parameter type cannot implicitly coerce to string * Document the criteria for marking a callable or tag as always allowed in a sandbox, and the list of built-in tags, filters, and functions that will be always allowed in Twig 4.0 - * Add an `always_allowed_in_sandbox` option for filters and functions, and an `isAlwaysAllowedInSandbox()` method for token parsers, to let authors mark callables and tags that are always allowed in sandbox mode without explicit allow-listing + * Add an `always_allowed_in_sandbox` option for filters, functions, and tests, and an `isAlwaysAllowedInSandbox()` method for token parsers, to let authors mark callables and tags that are always allowed in sandbox mode without explicit allow-listing + * Add an allow-list for tests to `Twig\Sandbox\SecurityPolicy`, with the safe built-in tests flagged as always allowed so they keep working without allow-listing # 3.27.1 (2026-05-30) diff --git a/doc/deprecated.rst b/doc/deprecated.rst index fd8561f8c..ec315e058 100644 --- a/doc/deprecated.rst +++ b/doc/deprecated.rst @@ -64,6 +64,10 @@ Nodes * Not passing a ``BodyNode`` instance as the body of a ``ModuleNode`` or ``MacroNode`` constructor is deprecated as of Twig 3.12. +* Not passing the ``$usedTests`` argument to + ``Twig\Node\CheckSecurityNode::__construct()`` is deprecated as of Twig + 3.28; the argument will be required in 4.0. + * Returning ``null`` from ``TokenParserInterface::parse()`` is deprecated as of Twig 3.12 (as forbidden by the interface). @@ -323,6 +327,28 @@ Sandbox parsers extending ``Twig\TokenParser\AbstractTokenParser`` inherit a default implementation that returns ``false``. +* Having the ``constant`` test and user-defined tests always allowed by default + in a sandbox is deprecated as of Twig 3.28. You will need to explicitly allow + them via the new ``allowedTests`` parameter of ``Twig\Sandbox\SecurityPolicy`` + (or via ``setAllowedTests()``) in 4.0. The same ``setStrict(true)`` toggle + opts-in to the 4.0 behavior for tests too. The other built-in tests + (``empty``, ``defined``, ``even``, ``same as``, ``iterable``, etc.) are + flagged as always allowed and do not trigger this deprecation. + +* Not declaring a 4th ``array $tests`` argument in + ``Twig\Sandbox\SecurityPolicyInterface::checkSecurity()`` implementations is + deprecated as of Twig 3.28. The argument will be part of the interface + signature in 4.0. + +* Not passing the ``$tests`` argument to + ``Twig\Sandbox\SecurityPolicy::checkSecurity()`` is deprecated as of Twig + 3.28; it will be required in 4.0. + +* Passing a ``Twig\Source`` as the 4th argument of + ``Twig\Extension\SandboxExtension::checkSecurity()`` is deprecated as of + Twig 3.28. The 4th argument is now an ``array`` of tests; pass the source + as the 5th argument instead. + * The ``Twig\Sandbox\SourcePolicyInterface`` interface is deprecated as of Twig 3.27.0 with no replacement. Passing an instance to the ``Twig\Extension\SandboxExtension`` constructor triggers a deprecation. diff --git a/doc/sandbox.rst b/doc/sandbox.rst index 09d1ebe46..9799cd005 100644 --- a/doc/sandbox.rst +++ b/doc/sandbox.rst @@ -29,13 +29,24 @@ properties and methods on objects:: 'Article' => ['title', 'body'], ]; $functions = ['range']; - $policy = new \Twig\Sandbox\SecurityPolicy($tags, $filters, $methods, $properties, $functions); + $tests = ['my_test']; + $policy = new \Twig\Sandbox\SecurityPolicy($tags, $filters, $methods, $properties, $functions, $tests); With the above configuration, the security policy will only allow usage of the -``if`` tag, and the ``upper`` filter. Moreover, the templates will only be able -to call the ``getTitle()`` and ``getBody()`` methods on ``Article`` objects, -and the ``title`` and ``body`` public properties. Everything else won't be -allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception. +``if`` tag, the ``upper`` filter, and the ``my_test`` test (on top of the +built-in tests that are always allowed, see below). Moreover, the templates +will only be able to call the ``getTitle()`` and ``getBody()`` methods on +``Article`` objects, and the ``title`` and ``body`` public properties. +Everything else won't be allowed and will generate a +``\Twig\Sandbox\SecurityError`` exception. + +.. note:: + + The ``allowedTests`` argument is available since Twig 3.28 (in earlier + versions all tests were always allowed). Most built-in tests (``empty``, + ``defined``, ``even``, ``same as``, ``iterable``, etc.) are always allowed + and do not need to be listed. Only custom tests and the built-in + ``constant`` test must be allow-listed like filters and functions. .. note:: @@ -48,28 +59,28 @@ allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception. .. caution:: - The ``extends`` and ``use`` tags, as well as the ``parent``, ``block``, and - ``attribute`` functions are always allowed in a sandboxed template. That - behavior will change in 4.0 where they will need to be explicitly allowed - like any other tag or function. To opt-in to the 4.0 behavior now (so they - need to be allow-listed or get rejected), enable strict mode on the - security policy:: + The ``extends`` and ``use`` tags, the ``parent``, ``block``, and + ``attribute`` functions, the ``constant`` test, and any custom test are + always allowed in a sandboxed template. That behavior will change in 4.0 + where they will need to be explicitly allowed like any other tag, filter, + function, or test. To opt-in to the 4.0 behavior now (so they need to be + allow-listed or get rejected), enable strict mode on the security policy:: $policy->setStrict(true); -Marking Filters, Functions, and Tags as Always Allowed ------------------------------------------------------- +Marking Filters, Functions, Tests, and Tags as Always Allowed +------------------------------------------------------------- .. versionadded:: 3.28 - The ``always_allowed_in_sandbox`` option for filters and functions, and - the ``isAlwaysAllowedInSandbox()`` method for token parsers, were added in - Twig 3.28. + The ``always_allowed_in_sandbox`` option for filters, functions, and tests, + and the ``isAlwaysAllowedInSandbox()`` method for token parsers, were added + in Twig 3.28. -Some filters, functions, and tags are inherently safe and should always be -usable in sandboxed templates without forcing every policy to allow-list them. -Mark such callables by setting the ``always_allowed_in_sandbox`` option to -``true``:: +Some filters, functions, tests, and tags are inherently safe and should always +be usable in sandboxed templates without forcing every policy to allow-list +them. Mark such callables by setting the ``always_allowed_in_sandbox`` option +to ``true``:: $twig->addFilter(new \Twig\TwigFilter('upper', 'strtoupper', [ 'always_allowed_in_sandbox' => true, @@ -79,6 +90,10 @@ Mark such callables by setting the ``always_allowed_in_sandbox`` option to 'always_allowed_in_sandbox' => true, ])); + $twig->addTest(new \Twig\TwigTest('even', null, [ + 'always_allowed_in_sandbox' => true, + ])); + For tags, override ``isAlwaysAllowedInSandbox()`` on your token parser to return ``true``:: @@ -92,8 +107,8 @@ return ``true``:: // ... } -Marked filters, functions, and tags are skipped by the sandbox security check -entirely, so they incur no runtime overhead, and they do not need to be +Marked filters, functions, tests, and tags are skipped by the sandbox security +check entirely, so they incur no runtime overhead, and they do not need to be listed in the ``SecurityPolicy`` allow-lists. The sandbox assumes that attackers control template source, not the Twig @@ -110,9 +125,9 @@ Only mark a callable or tag as always allowed when **all** the following conditions hold: * **No new capability.** The item must not expose anything beyond what the - sandbox already accepts. Pure value predicates (``is even``), pure value - transformations (``upper``, ``trim``, ``abs``), and pure control flow - (``if``, ``for``, ``set``) qualify. + sandbox already accepts. Pure value predicates (``is even``, ``is empty``), + pure value transformations (``upper``, ``trim``, ``abs``), and pure control + flow (``if``, ``for``, ``set``) qualify. * **No PHP runtime access.** The item must not read arbitrary PHP constants, call arbitrary classes or functions, instantiate objects from user-controlled names, or otherwise reach into the PHP runtime. This rules @@ -171,6 +186,15 @@ When upgrading to 4.0, you can drop these names from your ``SecurityPolicy`` allow-lists. Leaving them in is harmless: listing a name that is always allowed has no effect. +The corresponding built-in tests (``defined``, ``divisible by``, ``empty``, +``even``, ``iterable``, ``mapping``, ``none``, ``null``, ``odd``, ``same as``, +``sequence``, ``true``) are **already** flagged as always allowed since Twig +3.28, so they never need to be allow-listed. This is safe because tests were +never enforced by the sandbox before 3.28: flagging them keeps existing +templates working unchanged. The ``constant`` test is the exception: it reaches +into the PHP runtime, so it is not always allowed and must be allow-listed (it +is still implicitly allowed in 3.x with a deprecation, and rejected in 4.0). + Enabling the Sandbox -------------------- diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9b8c8d863..a62d67492 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,12 @@ parameters: ignoreErrors: + - # The "$tests" parameter is documented now and will be part of the signature in 4.0 + message: '#^PHPDoc tag @param references unknown parameter\: \$tests$#' + identifier: parameter.notFound + count: 1 + path: src/Sandbox/SecurityPolicyInterface.php + + - # 2 parameters will be required message: '#^Method Twig\\Node\\IncludeNode\:\:addGetTemplate\(\) invoked with 2 parameters, 1 required\.$#' identifier: arguments.count diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php index 2a09b6c7f..09b043274 100644 --- a/src/Extension/CoreExtension.php +++ b/src/Extension/CoreExtension.php @@ -310,19 +310,19 @@ final class CoreExtension extends AbstractExtension public function getTests(): array { return [ - new TwigTest('even', null, ['node_class' => EvenTest::class]), - new TwigTest('odd', null, ['node_class' => OddTest::class]), - new TwigTest('defined', null, ['node_class' => DefinedTest::class]), - new TwigTest('same as', null, ['node_class' => SameasTest::class, 'one_mandatory_argument' => true]), - new TwigTest('none', null, ['node_class' => NullTest::class]), - new TwigTest('null', null, ['node_class' => NullTest::class]), - new TwigTest('divisible by', null, ['node_class' => DivisiblebyTest::class, 'one_mandatory_argument' => true]), + new TwigTest('even', null, ['node_class' => EvenTest::class, 'always_allowed_in_sandbox' => true]), + new TwigTest('odd', null, ['node_class' => OddTest::class, 'always_allowed_in_sandbox' => true]), + new TwigTest('defined', null, ['node_class' => DefinedTest::class, 'always_allowed_in_sandbox' => true]), + new TwigTest('same as', null, ['node_class' => SameasTest::class, 'one_mandatory_argument' => true, 'always_allowed_in_sandbox' => true]), + new TwigTest('none', null, ['node_class' => NullTest::class, 'always_allowed_in_sandbox' => true]), + new TwigTest('null', null, ['node_class' => NullTest::class, 'always_allowed_in_sandbox' => true]), + new TwigTest('divisible by', null, ['node_class' => DivisiblebyTest::class, 'one_mandatory_argument' => true, 'always_allowed_in_sandbox' => true]), new TwigTest('constant', null, ['node_class' => ConstantTest::class]), - new TwigTest('empty', [self::class, 'testEmpty']), - new TwigTest('iterable', 'is_iterable'), - new TwigTest('sequence', [self::class, 'testSequence']), - new TwigTest('mapping', [self::class, 'testMapping']), - new TwigTest('true', null, ['node_class' => TrueTest::class]), + new TwigTest('empty', [self::class, 'testEmpty'], ['always_allowed_in_sandbox' => true]), + new TwigTest('iterable', 'is_iterable', ['always_allowed_in_sandbox' => true]), + new TwigTest('sequence', [self::class, 'testSequence'], ['always_allowed_in_sandbox' => true]), + new TwigTest('mapping', [self::class, 'testMapping'], ['always_allowed_in_sandbox' => true]), + new TwigTest('true', null, ['node_class' => TrueTest::class, 'always_allowed_in_sandbox' => true]), ]; } diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php index bdc3e7348..181e58c9c 100644 --- a/src/Extension/SandboxExtension.php +++ b/src/Extension/SandboxExtension.php @@ -86,11 +86,29 @@ final class SandboxExtension extends AbstractExtension return $this->policy; } - public function checkSecurity($tags, $filters, $functions, ?Source $source = null): void + public function checkSecurity($tags, $filters, $functions, $tests = [], $source = null): void { - if ($this->isSandboxed($source)) { - $this->policy->checkSecurity($tags, $filters, $functions); + // BC: previous signature was checkSecurity($tags, $filters, $functions, ?Source $source = null); + // detect a legacy call where the 4th positional argument was the Source. + if ($tests instanceof Source || (null === $tests && \func_num_args() < 5)) { + trigger_deprecation('twig/twig', '3.28', 'Passing a "Twig\Source" as the 4th argument of "%s()" is deprecated; pass an array of tests instead.', __METHOD__); + $source = $tests; + $tests = []; } + + if (!$this->isSandboxed($source)) { + return; + } + + if ((new \ReflectionMethod($this->policy, 'checkSecurity'))->getNumberOfParameters() >= 4) { + $this->policy->checkSecurity($tags, $filters, $functions, $tests); + + return; + } + + trigger_deprecation('twig/twig', '3.28', 'The "%s::checkSecurity()" method will take a 4th "array $tests" argument in 4.0; not declaring it is deprecated.', $this->policy::class); + + $this->policy->checkSecurity($tags, $filters, $functions); } public function checkMethodAllowed($obj, $method, int $lineno = -1, ?Source $source = null): void diff --git a/src/Node/CheckSecurityNode.php b/src/Node/CheckSecurityNode.php index d2aa5a864..92316aedc 100644 --- a/src/Node/CheckSecurityNode.php +++ b/src/Node/CheckSecurityNode.php @@ -23,17 +23,24 @@ class CheckSecurityNode extends Node private $usedFilters; private $usedTags; private $usedFunctions; + private $usedTests; /** * @param array $usedFilters * @param array $usedTags * @param array $usedFunctions + * @param array $usedTests */ - public function __construct(array $usedFilters, array $usedTags, array $usedFunctions) + public function __construct(array $usedFilters, array $usedTags, array $usedFunctions, array $usedTests = []) { + if (\func_num_args() < 4) { + trigger_deprecation('twig/twig', '3.28', 'Not passing the "$usedTests" argument to "%s::__construct()" is deprecated; it will be required in 4.0.', static::class); + } + $this->usedFilters = $usedFilters; $this->usedTags = $usedTags; $this->usedFunctions = $usedFunctions; + $this->usedTests = $usedTests; parent::__construct(); } @@ -58,7 +65,8 @@ class CheckSecurityNode extends Node ->indent() ->write('static $tags = ')->repr(array_filter($this->usedTags))->raw(";\n") ->write('static $filters = ')->repr(array_filter($this->usedFilters))->raw(";\n") - ->write('static $functions = ')->repr(array_filter($this->usedFunctions))->raw(";\n\n") + ->write('static $functions = ')->repr(array_filter($this->usedFunctions))->raw(";\n") + ->write('static $tests = ')->repr(array_filter($this->usedTests))->raw(";\n\n") ->write("try {\n") ->indent() ->write("\$this->sandbox->checkSecurity(\n") @@ -66,6 +74,7 @@ class CheckSecurityNode extends Node ->write('')->repr(array_keys($this->usedTags))->raw(",\n") ->write('')->repr(array_keys($this->usedFilters))->raw(",\n") ->write('')->repr(array_keys($this->usedFunctions))->raw(",\n") + ->write('')->repr(array_keys($this->usedTests))->raw(",\n") ->write("\$this->source\n") ->outdent() ->write(");\n") @@ -85,6 +94,10 @@ class CheckSecurityNode extends Node ->indent() ->write("\$e->setTemplateLine(\$functions[\$e->getFunctionName()]);\n") ->outdent() + ->write("} elseif (\$e instanceof SecurityNotAllowedTestError && isset(\$tests[\$e->getTestName()])) {\n") + ->indent() + ->write("\$e->setTemplateLine(\$tests[\$e->getTestName()]);\n") + ->outdent() ->write("}\n\n") ->write("throw \$e;\n") ->outdent() diff --git a/src/Node/Expression/Binary/NullCoalesceBinary.php b/src/Node/Expression/Binary/NullCoalesceBinary.php index a047b6030..ffba62dc6 100644 --- a/src/Node/Expression/Binary/NullCoalesceBinary.php +++ b/src/Node/Expression/Binary/NullCoalesceBinary.php @@ -32,12 +32,12 @@ final class NullCoalesceBinary extends AbstractBinary implements OperatorEscapeI { parent::__construct($left, $right, $lineno); - $test = new DefinedTest(clone $left, new TwigTest('defined'), new EmptyNode(), $left->getTemplateLine()); + $test = new DefinedTest(clone $left, new TwigTest('defined', null, ['always_allowed_in_sandbox' => true]), new EmptyNode(), $left->getTemplateLine()); // for "block()", we don't need the null test as the return value is always a string if (!$left instanceof BlockReferenceExpression) { $test = new AndBinary( $test, - new NotUnary(new NullTest($left, new TwigTest('null'), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()), + new NotUnary(new NullTest($left, new TwigTest('null', null, ['always_allowed_in_sandbox' => true]), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()), $left->getTemplateLine(), ); } diff --git a/src/Node/Expression/Filter/DefaultFilter.php b/src/Node/Expression/Filter/DefaultFilter.php index 04ef06cc4..fc64b88a9 100644 --- a/src/Node/Expression/Filter/DefaultFilter.php +++ b/src/Node/Expression/Filter/DefaultFilter.php @@ -54,7 +54,7 @@ class DefaultFilter extends FilterExpression } if ('default' === $name && ($node instanceof ContextVariable || $node instanceof GetAttrExpression)) { - $test = new DefinedTest(clone $node, new TwigTest('defined'), new EmptyNode(), $node->getTemplateLine()); + $test = new DefinedTest(clone $node, new TwigTest('defined', null, ['always_allowed_in_sandbox' => true]), new EmptyNode(), $node->getTemplateLine()); $false = \count($arguments) ? $arguments->getNode('0') : new ConstantExpression('', $node->getTemplateLine()); $node = new ConditionalTernary($test, $default, $false, $node->getTemplateLine()); diff --git a/src/Node/Expression/NullCoalesceExpression.php b/src/Node/Expression/NullCoalesceExpression.php index f397f71f0..4548519f5 100644 --- a/src/Node/Expression/NullCoalesceExpression.php +++ b/src/Node/Expression/NullCoalesceExpression.php @@ -39,12 +39,12 @@ class NullCoalesceExpression extends ConditionalExpression trigger_deprecation('twig/twig', '3.15', 'Not passing a "%s" instance to the "right" argument of "%s" is deprecated ("%s" given).', AbstractExpression::class, static::class, $right::class); } - $test = new DefinedTest(clone $left, new TwigTest('defined'), new EmptyNode(), $left->getTemplateLine()); + $test = new DefinedTest(clone $left, new TwigTest('defined', null, ['always_allowed_in_sandbox' => true]), new EmptyNode(), $left->getTemplateLine()); // for "block()", we don't need the null test as the return value is always a string if (!$left instanceof BlockReferenceExpression) { $test = new AndBinary( $test, - new NotUnary(new NullTest($left, new TwigTest('null'), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()), + new NotUnary(new NullTest($left, new TwigTest('null', null, ['always_allowed_in_sandbox' => true]), new EmptyNode(), $left->getTemplateLine()), $left->getTemplateLine()), $left->getTemplateLine() ); } diff --git a/src/Node/Expression/Ternary/ConditionalTernary.php b/src/Node/Expression/Ternary/ConditionalTernary.php index f7cd78c5c..7a09189ac 100644 --- a/src/Node/Expression/Ternary/ConditionalTernary.php +++ b/src/Node/Expression/Ternary/ConditionalTernary.php @@ -23,7 +23,7 @@ final class ConditionalTernary extends AbstractExpression implements OperatorEsc public function __construct(AbstractExpression $test, AbstractExpression $left, AbstractExpression $right, int $lineno) { if (!$test instanceof ReturnPrimitiveTypeInterface) { - $test = new TrueTest($test, new TwigTest('true'), null, $test->getTemplateLine()); + $test = new TrueTest($test, new TwigTest('true', null, ['always_allowed_in_sandbox' => true]), null, $test->getTemplateLine()); } parent::__construct(['test' => $test, 'left' => $left, 'right' => $right], [], $lineno); diff --git a/src/Node/IfNode.php b/src/Node/IfNode.php index 2c0e2a8e9..1f91e0058 100644 --- a/src/Node/IfNode.php +++ b/src/Node/IfNode.php @@ -31,7 +31,7 @@ class IfNode extends Node for ($i = 0, $count = \count($tests); $i < $count; $i += 2) { $test = $tests->getNode((string) $i); if (!$test instanceof ReturnPrimitiveTypeInterface) { - $tests->setNode($i, new TrueTest($test, new TwigTest('true'), null, $test->getTemplateLine())); + $tests->setNode($i, new TrueTest($test, new TwigTest('true', null, ['always_allowed_in_sandbox' => true]), null, $test->getTemplateLine())); } } $nodes = ['tests' => $tests]; diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index b2529e9fc..fd43246f4 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -184,6 +184,7 @@ final class ModuleNode extends Node implements CoercesChildrenToStringInterface ->write("use Twig\Sandbox\SecurityNotAllowedTagError;\n") ->write("use Twig\Sandbox\SecurityNotAllowedFilterError;\n") ->write("use Twig\Sandbox\SecurityNotAllowedFunctionError;\n") + ->write("use Twig\Sandbox\SecurityNotAllowedTestError;\n") ->write("use Twig\Source;\n") ->write("use Twig\Template;\n") ->write("use Twig\TemplateWrapper;\n") diff --git a/src/NodeVisitor/SandboxNodeVisitor.php b/src/NodeVisitor/SandboxNodeVisitor.php index b0e4e748a..dba49c8b0 100644 --- a/src/NodeVisitor/SandboxNodeVisitor.php +++ b/src/NodeVisitor/SandboxNodeVisitor.php @@ -22,6 +22,7 @@ use Twig\Node\Expression\FilterExpression; use Twig\Node\Expression\FunctionExpression; use Twig\Node\Expression\GetAttrExpression; use Twig\Node\Expression\OperatorEscapeInterface; +use Twig\Node\Expression\TestExpression; use Twig\Node\Expression\Unary\SpreadUnary; use Twig\Node\Expression\Variable\ContextVariable; use Twig\Node\ModuleNode; @@ -45,6 +46,8 @@ final class SandboxNodeVisitor implements NodeVisitorInterface private $filters; /** @var array */ private $functions; + /** @var array */ + private $tests; public function enterNode(Node $node, Environment $env): Node { @@ -53,6 +56,7 @@ final class SandboxNodeVisitor implements NodeVisitorInterface $this->tags = []; $this->filters = []; $this->functions = []; + $this->tests = []; } elseif ($this->inAModule) { // look for tags if ($node->getNodeTag() && !isset($this->tags[$node->getNodeTag()]) && !$this->isTagAlwaysAllowedInSandbox($env, $node->getNodeTag())) { @@ -69,6 +73,11 @@ final class SandboxNodeVisitor implements NodeVisitorInterface $this->functions[$name] = $node->getTemplateLine(); } + // look for tests + if ($node instanceof TestExpression && !isset($this->tests[$name = $node->getAttribute('name')]) && !$this->isTestAlwaysAllowedInSandbox($env, $node)) { + $this->tests[$name] = $node->getTemplateLine(); + } + // look for functions whose parser callable replaced the FunctionExpression // with a specialized node (e.g. `parent`, `block`, `attribute`); the // original function name was stashed by FunctionExpressionParser. @@ -117,7 +126,7 @@ final class SandboxNodeVisitor implements NodeVisitorInterface $this->inAModule = false; $node->setNode('constructor_end', new Nodes([new CheckSecurityCallNode(), $node->getNode('constructor_end')])); - $node->setNode('class_end', new Nodes([new CheckSecurityNode($this->filters, $this->tags, $this->functions), $node->getNode('class_end')])); + $node->setNode('class_end', new Nodes([new CheckSecurityNode($this->filters, $this->tags, $this->functions, $this->tests), $node->getNode('class_end')])); } return $node; @@ -233,6 +242,17 @@ final class SandboxNodeVisitor implements NodeVisitorInterface return self::isAlwaysAllowedInSandbox($function); } + private function isTestAlwaysAllowedInSandbox(Environment $env, TestExpression $node): bool + { + if ($node->hasAttribute('twig_callable')) { + $test = $node->getAttribute('twig_callable'); + } elseif (null === $test = $env->getTest($node->getAttribute('name'))) { + return false; + } + + return self::isAlwaysAllowedInSandbox($test); + } + private function isSandboxedFunctionAlwaysAllowedInSandbox(Environment $env, Node $node, string $name): bool { if ($node->hasAttribute('sandboxed_function')) { diff --git a/src/Sandbox/SecurityNotAllowedTestError.php b/src/Sandbox/SecurityNotAllowedTestError.php new file mode 100644 index 000000000..af702ba68 --- /dev/null +++ b/src/Sandbox/SecurityNotAllowedTestError.php @@ -0,0 +1,33 @@ + + */ +final class SecurityNotAllowedTestError extends SecurityError +{ + private string $testName; + + public function __construct(string $message, string $testName) + { + parent::__construct($message); + $this->testName = $testName; + } + + public function getTestName(): string + { + return $this->testName; + } +} diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php index 061b7974d..7d1172bf1 100644 --- a/src/Sandbox/SecurityPolicy.php +++ b/src/Sandbox/SecurityPolicy.php @@ -26,15 +26,17 @@ final class SecurityPolicy implements SecurityPolicyInterface private $allowedMethods; private $allowedProperties; private $allowedFunctions; + private array $allowedTests; private bool $strict = false; - public function __construct(array $allowedTags = [], array $allowedFilters = [], array $allowedMethods = [], array $allowedProperties = [], array $allowedFunctions = []) + public function __construct(array $allowedTags = [], array $allowedFilters = [], array $allowedMethods = [], array $allowedProperties = [], array $allowedFunctions = [], array $allowedTests = []) { $this->allowedTags = $allowedTags; $this->allowedFilters = $allowedFilters; $this->setAllowedMethods($allowedMethods); $this->allowedProperties = $allowedProperties; $this->allowedFunctions = $allowedFunctions; + $this->allowedTests = $allowedTests; } public function setAllowedTags(array $tags): void @@ -65,22 +67,32 @@ final class SecurityPolicy implements SecurityPolicyInterface $this->allowedFunctions = $functions; } + public function setAllowedTests(array $tests): void + { + $this->allowedTests = $tests; + } + /** * Toggles strict mode. * - * In strict mode, the tags and functions that are historically always allowed in a - * sandbox (the ``extends`` and ``use`` tags, the ``parent``, ``block``, and - * ``attribute`` functions) are no longer implicitly allowed and must be added to the - * relevant allow-list to be usable. Use this flag in 3.x to opt-in to the forthcoming - * 4.0 behavior and silence the related deprecations. + * In strict mode, the tags, functions, and tests that are historically always + * allowed in a sandbox (the ``extends`` and ``use`` tags, the ``parent``, + * ``block``, and ``attribute`` functions, and any test) are no longer implicitly + * allowed and must be added to the relevant allow-list to be usable. Use this + * flag in 3.x to opt-in to the forthcoming 4.0 behavior and silence the related + * deprecations. */ public function setStrict(bool $strict): void { $this->strict = $strict; } - public function checkSecurity($tags, $filters, $functions): void + public function checkSecurity($tags, $filters, $functions, array $tests = []): void { + if (\func_num_args() < 4) { + trigger_deprecation('twig/twig', '3.28', 'Not passing the "$tests" argument to "%s::checkSecurity()" is deprecated; it will be required in 4.0.', static::class); + } + foreach ($tags as $tag) { if (!\in_array($tag, $this->allowedTags, true)) { if (!$this->strict && 'extends' === $tag) { @@ -112,6 +124,16 @@ final class SecurityPolicy implements SecurityPolicyInterface } } } + + foreach ($tests as $test) { + if (!\in_array($test, $this->allowedTests, true)) { + if (!$this->strict) { + trigger_deprecation('twig/twig', '3.28', 'The "%s" test is always allowed in sandboxes, but won\'t be in 4.0, please enable it explicitly in your sandbox policy if needed (or enable strict mode on the security policy to opt-in to the 4.0 behavior now).', $test); + } else { + throw new SecurityNotAllowedTestError(\sprintf('Test "%s" is not allowed.', $test), $test); + } + } + } } public function checkMethodAllowed($obj, $method): void diff --git a/src/Sandbox/SecurityPolicyInterface.php b/src/Sandbox/SecurityPolicyInterface.php index 36471c54c..cb36ba5be 100644 --- a/src/Sandbox/SecurityPolicyInterface.php +++ b/src/Sandbox/SecurityPolicyInterface.php @@ -22,10 +22,11 @@ interface SecurityPolicyInterface * @param string[] $tags * @param string[] $filters * @param string[] $functions + * @param string[] $tests * * @throws SecurityError */ - public function checkSecurity($tags, $filters, $functions): void; + public function checkSecurity($tags, $filters, $functions/* , array $tests */): void; /** * @param object $obj diff --git a/tests/Extension/SandboxStateChangeTest.php b/tests/Extension/SandboxStateChangeTest.php index f6981517c..f6b1fcc6e 100644 --- a/tests/Extension/SandboxStateChangeTest.php +++ b/tests/Extension/SandboxStateChangeTest.php @@ -413,10 +413,10 @@ class CountingSecurityPolicy implements SecurityPolicyInterface { } - public function checkSecurity($tags, $filters, $functions): void + public function checkSecurity($tags, $filters, $functions, array $tests = []): void { ++$this->callCount; - $this->inner->checkSecurity($tags, $filters, $functions); + $this->inner->checkSecurity($tags, $filters, $functions, $tests); } public function checkMethodAllowed($obj, $method): void diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index bffe3e053..843c25876 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -42,7 +42,9 @@ use Twig\Sandbox\SecurityNotAllowedFunctionError; use Twig\Sandbox\SecurityNotAllowedMethodError; use Twig\Sandbox\SecurityNotAllowedPropertyError; use Twig\Sandbox\SecurityNotAllowedTagError; +use Twig\Sandbox\SecurityNotAllowedTestError; use Twig\Sandbox\SecurityPolicy; +use Twig\Sandbox\SecurityPolicyInterface; use Twig\Sandbox\SourcePolicyInterface; use Twig\Source; use Twig\Token; @@ -357,7 +359,208 @@ class SandboxTest extends TestCase $policy->setStrict(true); $this->expectException(SecurityNotAllowedTagError::class); - $policy->checkSecurity(['extends'], [], []); + $policy->checkSecurity(['extends'], [], [], []); + } + + /** + * @dataProvider getAlwaysAllowedCoreTests + */ + #[DataProvider('getAlwaysAllowedCoreTests')] + public function testSandboxAllowsAlwaysAllowedCoreTests(string $template) + { + // the safe built-in tests are always allowed in a sandbox (they carry + // the `always_allowed_in_sandbox` flag), so they need neither an + // allow-list entry nor strict mode to be opted out of + $twig = $this->getEnvironment(true, [], self::$templates, [], [], [], [], [], null, true); + + $this->assertSame('y', $twig->createTemplate($template, 'index')->render([])); + } + + public static function getAlwaysAllowedCoreTests() + { + yield ['{{ "" is empty ? "y" }}']; + yield ['{{ [] is iterable ? "y" }}']; + yield ['{{ null is null ? "y" }}']; + yield ['{{ null is none ? "y" }}']; + yield ['{{ 1 is defined ? "y" }}']; + yield ['{{ 2 is even ? "y" }}']; + yield ['{{ 3 is odd ? "y" }}']; + yield ['{{ true is true ? "y" }}']; + yield ['{{ 1 is same as(1) ? "y" }}']; + yield ['{{ 4 is divisible by(2) ? "y" }}']; + yield ['{{ [] is sequence ? "y" }}']; + yield ['{{ {"a": 1} is mapping ? "y" }}']; + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testSandboxForConstantTest() + { + // unlike the other built-in tests, "constant" reaches into the PHP + // runtime, so it is not always allowed and is deprecated until 4.0 + $this->expectDeprecation('Since twig/twig 3.28: The "constant" test is always allowed in sandboxes, but won\'t be in 4.0, please enable it explicitly in your sandbox policy if needed (or enable strict mode on the security policy to opt-in to the 4.0 behavior now).'); + + $twig = $this->getEnvironment(true, [], ['index' => '{{ 1 is constant("PHP_INT_MAX") ? "y" }}']); + $twig->load('index')->render([]); + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testSandboxForUserDefinedTest() + { + $this->expectDeprecation('Since twig/twig 3.28: The "unsafe" test is always allowed in sandboxes, but won\'t be in 4.0, please enable it explicitly in your sandbox policy if needed (or enable strict mode on the security policy to opt-in to the 4.0 behavior now).'); + + $called = 0; + $twig = $this->getEnvironment(true, [], ['index' => '{{ "x" is unsafe ? "y" }}']); + $twig->addTest(new TwigTest('unsafe', static function ($value) use (&$called): bool { + ++$called; + + return true; + })); + + $this->assertSame('y', $twig->load('index')->render([])); + $this->assertSame(1, $called); + } + + public function testSandboxAllowsAllowListedTest() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ "x" is unsafe ? "y" }}'], [], [], [], [], [], null, false, ['unsafe']); + $twig->addTest(new TwigTest('unsafe', static fn ($v): bool => true)); + + $this->assertSame('y', $twig->load('index')->render([])); + } + + public function testStrictSandboxRejectsConstantTest() + { + // "constant" is the only built-in test that is not always allowed + $twig = $this->getEnvironment(true, [], ['index' => '{{ 1 is constant("PHP_INT_MAX") ? "y" }}'], [], [], [], [], [], null, true); + + $this->expectException(SecurityNotAllowedTestError::class); + $this->expectExceptionMessage('Test "constant" is not allowed'); + + $twig->load('index')->render([]); + } + + public function testStrictSandboxStillAllowsAllowListedTest() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ 1 is constant("PHP_INT_MAX") ? "y" : "n" }}'], [], [], [], [], [], null, true, ['constant']); + + $this->assertSame('n', $twig->load('index')->render([])); + } + + public function testStrictSandboxRejectsUserDefinedTest() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ "x" is unsafe ? "y" }}'], [], [], [], [], [], null, true); + $twig->addTest(new TwigTest('unsafe', static fn ($v): bool => true)); + + $this->expectException(SecurityNotAllowedTestError::class); + $this->expectExceptionMessage('Test "unsafe" is not allowed'); + + $twig->load('index')->render([]); + } + + public function testImplicitBooleanCoercionDoesNotRequireAllowListingTests() + { + // `{% if %}`, ternary, `?:`, `??`, and `|default` conditions are coerced + // to a boolean through `true`/`defined`/`null` tests that the compiler + // injects; the template author never wrote them and they are always + // allowed, so they must work even in strict mode with no test allow-listed + $template = "{% if x %}a{% endif %}{{ y ? 'b' : 'c' }}{{ z ?: 'd' }}{{ w ?? 'e' }}{{ v|default('f') }}"; + $twig = $this->getEnvironment(true, [], ['index' => $template], ['if'], ['default'], [], [], [], null, true); + + $this->assertSame('acdef', $twig->load('index')->render(['x' => true, 'y' => false])); + } + + public function testStrictSandboxRejectedTestCarriesSourceAndLine() + { + $twig = $this->getEnvironment(true, [], ['index' => "{{ 1 }}\n{{ 1 is constant('PHP_INT_MAX') ? 'y' }}"], [], [], [], [], [], null, true); + + try { + $twig->load('index')->render([]); + $this->fail('Expected SecurityNotAllowedTestError'); + } catch (SecurityNotAllowedTestError $e) { + $this->assertSame('constant', $e->getTestName()); + $this->assertSame(2, $e->getTemplateLine()); + $this->assertSame('index', $e->getSourceContext()->getName()); + } + } + + public function testStrictModeRejectsTestsViaSetter() + { + $policy = new SecurityPolicy([], [], [], [], []); + $policy->setStrict(true); + + $this->expectException(SecurityNotAllowedTestError::class); + $policy->checkSecurity([], [], [], ['empty']); + } + + public function testAllowedTestsCanBeUpdatedViaSetter() + { + $policy = new SecurityPolicy([], [], [], [], []); + $policy->setStrict(true); + $policy->setAllowedTests(['empty']); + + // does not throw + $policy->checkSecurity([], [], [], ['empty']); + + $this->expectException(SecurityNotAllowedTestError::class); + $policy->checkSecurity([], [], [], ['null']); + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testLegacySecurityPolicyWithoutTestsParameterTriggersDeprecation() + { + $this->expectDeprecation('Since twig/twig 3.28: The "Twig\Tests\Extension\LegacySandboxSecurityPolicy::checkSecurity()" method will take a 4th "array $tests" argument in 4.0; not declaring it is deprecated.'); + + $loader = new ArrayLoader(['index' => '{{ "x" is unsafe ? "y" }}']); + $twig = new Environment($loader, ['debug' => true, 'cache' => false, 'autoescape' => false]); + $twig->addExtension(new SandboxExtension(new LegacySandboxSecurityPolicy(), true)); + $twig->addTest(new TwigTest('unsafe', static fn ($v): bool => true)); + + // legacy policies silently allow tests (no security regression vs. today) + $this->assertSame('y', $twig->load('index')->render([])); + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testLegacyCheckSecurityCallWithSourceAs4thArgumentTriggersDeprecation() + { + $this->expectDeprecation('Since twig/twig 3.28: Passing a "Twig\Source" as the 4th argument of "Twig\Extension\SandboxExtension::checkSecurity()" is deprecated; pass an array of tests instead.'); + + $policy = new SecurityPolicy([], [], [], [], []); + $ext = new SandboxExtension($policy, true); + $ext->checkSecurity([], [], [], new Source('', 'index')); + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testLegacyCheckSecurityNodeWithoutUsedTestsTriggersDeprecation() + { + $this->expectDeprecation('Since twig/twig 3.28: Not passing the "$usedTests" argument to "Twig\Node\CheckSecurityNode::__construct()" is deprecated; it will be required in 4.0.'); + + new \Twig\Node\CheckSecurityNode([], [], []); + } + + /** + * @group legacy + */ + #[Group('legacy')] + public function testLegacySecurityPolicyCheckSecurityWithoutTestsArgTriggersDeprecation() + { + $this->expectDeprecation('Since twig/twig 3.28: Not passing the "$tests" argument to "Twig\Sandbox\SecurityPolicy::checkSecurity()" is deprecated; it will be required in 4.0.'); + + (new SecurityPolicy([], [], [], [], []))->checkSecurity([], [], []); } public function testSandboxWithInheritance() @@ -749,7 +952,7 @@ class SandboxTest extends TestCase public function testSandboxBlocksToStringOnIsConstantTestArgument() { - $twig = $this->getEnvironment(true, [], ['index' => '{% if "x" is constant(obj) %}LEAK{% endif %}'], ['if']); + $twig = $this->getEnvironment(true, [], ['index' => '{% if "x" is constant(obj) %}LEAK{% endif %}'], ['if'], [], [], [], [], null, false, ['constant']); try { $twig->load('index')->render(self::$params); $this->fail('Sandbox throws a SecurityError exception if __toString is called on a constant test argument'); @@ -788,7 +991,7 @@ class SandboxTest extends TestCase #[DataProvider('getSandboxAllowedToStringTests')] public function testSandboxAllowedToString($template, $output) { - $twig = $this->getEnvironment(true, [], ['index' => $template], ['set', 'do'], [], ['Twig\Tests\Extension\FooObject' => ['foo', 'getAnotherFooObject']]); + $twig = $this->getEnvironment(true, [], ['index' => $template], ['set', 'do'], [], ['Twig\Tests\Extension\FooObject' => ['foo', 'getAnotherFooObject']], [], [], null, false, ['constant']); $this->assertEquals($output, $twig->load('index')->render(self::$params)); } @@ -1560,6 +1763,7 @@ EOF // __toString wrap and bypass the policy. $twig = $this->getEnvironment(true, [], ['index' => '{{ 5 is my_test(obj) }}']); $twig->addTest(new TwigTest('my_test', static fn (int $value, $arg) => 'x' === (string) $arg)); + $twig->getExtension(SandboxExtension::class)->getSecurityPolicy()->setAllowedTests(['my_test']); try { $twig->load('index')->render(['obj' => new FooObject()]); @@ -1639,6 +1843,7 @@ EOF { $twig = $this->getEnvironment(true, [], ['index' => '{{ 1 is unsafe_test(foo_bar: obj) ? "yes" : "no" }}']); $twig->addTest(new TwigTest('unsafe_test', static fn ($value, $fooBar, int ...$rest) => 'x' === (string) $fooBar, ['is_variadic' => true])); + $twig->getExtension(SandboxExtension::class)->getSecurityPolicy()->setAllowedTests(['unsafe_test']); try { $twig->load('index')->render(['obj' => new FooObject()]); @@ -1771,11 +1976,11 @@ EOF } } - protected function getEnvironment($sandboxed, $options, $templates, $tags = [], $filters = [], $methods = [], $properties = [], $functions = [], $sourcePolicy = null, bool $strict = false) + protected function getEnvironment($sandboxed, $options, $templates, $tags = [], $filters = [], $methods = [], $properties = [], $functions = [], $sourcePolicy = null, bool $strict = false, array $tests = []) { $loader = new ArrayLoader($templates); $twig = new Environment($loader, array_merge(['debug' => true, 'cache' => false, 'autoescape' => false], $options)); - $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); + $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions, $tests); $policy->setStrict($strict); $twig->addExtension(new SandboxExtension($policy, $sandboxed, $sourcePolicy)); @@ -1967,7 +2172,7 @@ EOF public function testNeedsIsSandboxedTestReceivesTrueWhenSandboxed() { - $twig = $this->getEnvironment(true, [], ['index' => '{{ "foo" is sandbox_aware ? "on" : "off" }}']); + $twig = $this->getEnvironment(true, [], ['index' => '{{ "foo" is sandbox_aware ? "on" : "off" }}'], [], [], [], [], [], null, false, ['sandbox_aware']); $twig->addTest(new TwigTest('sandbox_aware', static function (bool $isSandboxed, string $value) { return $isSandboxed && 'foo' === $value; }, ['needs_is_sandboxed' => true])); @@ -2034,6 +2239,24 @@ EOF $twig->load('index')->render([]); } + public function testAlwaysAllowedInSandboxTestBypassesAllowList() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ 4 is safe_even ? "yes" : "no" }}'], [], [], [], [], [], null, true); + $twig->addTest(new TwigTest('safe_even', static fn ($value) => 0 === $value % 2, ['always_allowed_in_sandbox' => true])); + + $this->assertSame('yes', $twig->load('index')->render([])); + } + + public function testAlwaysAllowedInSandboxTestStillEnforcedWhenFlagNotSet() + { + $twig = $this->getEnvironment(true, [], ['index' => '{{ 4 is gated_even ? "yes" : "no" }}'], [], [], [], [], [], null, true); + $twig->addTest(new TwigTest('gated_even', static fn ($value) => 0 === $value % 2)); + + $this->expectException(SecurityNotAllowedTestError::class); + $this->expectExceptionMessage('Test "gated_even" is not allowed'); + $twig->load('index')->render([]); + } + public function testAlwaysAllowedInSandboxFunctionAlsoCoversRangeOperator() { $twig = $this->getEnvironment(true, [], ['index' => '{{ (1..2)[0] }}']); @@ -2437,3 +2660,18 @@ class LegacyTokenParserWithoutIsAlwaysAllowedInSandbox implements TokenParserInt return 'legacy_tag'; } } + +class LegacySandboxSecurityPolicy implements SecurityPolicyInterface +{ + public function checkSecurity($tags, $filters, $functions): void + { + } + + public function checkMethodAllowed($obj, $method): void + { + } + + public function checkPropertyAllowed($obj, $property): void + { + } +} diff --git a/tests/Node/ModuleTest.php b/tests/Node/ModuleTest.php index b8df54f77..9b8331aea 100644 --- a/tests/Node/ModuleTest.php +++ b/tests/Node/ModuleTest.php @@ -107,6 +107,7 @@ use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; use Twig\Sandbox\SecurityNotAllowedFilterError; use Twig\Sandbox\SecurityNotAllowedFunctionError; +use Twig\Sandbox\SecurityNotAllowedTestError; use Twig\Source; use Twig\Template; use Twig\TemplateWrapper; @@ -153,7 +154,7 @@ class __TwigTemplate_%x extends Template */ public function getDebugInfo(): array { - return array ( 42 => 1,); + return array ( 43 => 1,); } public function getSourceContext(): Source @@ -182,6 +183,7 @@ use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; use Twig\Sandbox\SecurityNotAllowedFilterError; use Twig\Sandbox\SecurityNotAllowedFunctionError; +use Twig\Sandbox\SecurityNotAllowedTestError; use Twig\Source; use Twig\Template; use Twig\TemplateWrapper; @@ -242,7 +244,7 @@ class __TwigTemplate_%x extends Template */ public function getDebugInfo(): array { - return array ( 48 => 1, 46 => 2, 39 => 1,); + return array ( 49 => 1, 47 => 2, 40 => 1,); } public function getSourceContext(): Source @@ -276,6 +278,7 @@ use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; use Twig\Sandbox\SecurityNotAllowedFilterError; use Twig\Sandbox\SecurityNotAllowedFunctionError; +use Twig\Sandbox\SecurityNotAllowedTestError; use Twig\Source; use Twig\Template; use Twig\TemplateWrapper; @@ -335,7 +338,7 @@ class __TwigTemplate_%x extends Template */ public function getDebugInfo(): array { - return array ( 48 => 2, 46 => 4, 39 => 2,); + return array ( 49 => 2, 47 => 4, 40 => 2,); } public function getSourceContext(): Source