Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: unblock
Please unblock package twig, it backports a security fix (Sandbox Information Disclosure) from the latest (2.7) version. https://symfony.com/blog/twig-sandbox-information-disclosure Unfortunately, upstream moved from PSR-0 to PSR-4 prior to fixing this security issue, so I had to backport the fix instead of simply cherry-pick the commit. I managed to backport the fixes of the testsuite too to help in the confidence that the fix is correct. 2.7 is in experimental, I can upload this version to unstable if you prefer. Ditto, upstream 1.38 moved from PSR-0 to PSR-4, and backporting the fix to 1.24 is even more tedious (some structures seem to have changed in between), so I’m not yet proposing a stretch-update (the security-team is X-Debbugs-CCed on this report, so they can share their point of view on this request). unblock twig/2.6.2-2 Thanks in advance. Regards David
diff --git a/debian/changelog b/debian/changelog index 60645e8a..446f5dfd 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +twig (2.6.2-2) unstable; urgency=medium + + * Team upload + * Stick to 2.6 for buster + * Backport fix from 2.7: security issue in the sandbox + + -- David Prévot <taf...@debian.org> Tue, 12 Mar 2019 10:35:44 -1000 + twig (2.6.2-1) unstable; urgency=medium * Team upload diff --git a/debian/gbp.conf b/debian/gbp.conf index cec628c7..f7127058 100644 --- a/debian/gbp.conf +++ b/debian/gbp.conf @@ -1,2 +1,3 @@ [DEFAULT] +debian-branch = buster pristine-tar = True diff --git a/debian/patches/0001-Fix-security-issue-in-the-sandbox.patch b/debian/patches/0001-Fix-security-issue-in-the-sandbox.patch new file mode 100644 index 00000000..7f872fc0 --- /dev/null +++ b/debian/patches/0001-Fix-security-issue-in-the-sandbox.patch @@ -0,0 +1,346 @@ +From: =?utf-8?q?David_Pr=C3=A9vot?= <da...@tilapin.org> +Date: Tue, 12 Mar 2019 10:13:15 -1000 +Subject: Fix security issue in the sandbox + +Fix sandbox security issue (under some circumstances, calling the +__toString() method on an object was possible even if not allowed by the +security policy). + +Origin: backport, https://github.com/twigphp/Twig/commit/eac5422956e1dcca89a3669a03a3ff32f0502077 +--- + lib/Twig/Node/CheckToString.php | 39 ++++++++++++ + lib/Twig/Node/SandboxedPrint.php | 2 + + lib/Twig/NodeVisitor/Sandbox.php | 45 +++++++++++++- + src/Node/CheckToStringNode.php | 11 ++++ + test/Twig/Tests/Extension/SandboxTest.php | 95 ++++++++++++++++++++--------- + test/Twig/Tests/Node/SandboxedPrintTest.php | 33 ---------- + 6 files changed, 160 insertions(+), 65 deletions(-) + create mode 100644 lib/Twig/Node/CheckToString.php + create mode 100644 src/Node/CheckToStringNode.php + delete mode 100644 test/Twig/Tests/Node/SandboxedPrintTest.php + +diff --git a/lib/Twig/Node/CheckToString.php b/lib/Twig/Node/CheckToString.php +new file mode 100644 +index 0000000..07a7837 +--- /dev/null ++++ b/lib/Twig/Node/CheckToString.php +@@ -0,0 +1,39 @@ ++<?php ++ ++/* ++ * This file is part of Twig. ++ * ++ * (c) Fabien Potencier ++ * ++ * For the full copyright and license information, please view the LICENSE ++ * file that was distributed with this source code. ++ */ ++ ++/** ++ * Checks if casting an expression to __toString() is allowed by the sandbox. ++ * ++ * For instance, when there is a simple Print statement, like {{ article }}, ++ * and if the sandbox is enabled, we need to check that the __toString() ++ * method is allowed if 'article' is an object. The same goes for {{ article|upper }} ++ * or {{ random(article) }} ++ * ++ * @author Fabien Potencier <fab...@symfony.com> ++ */ ++class Twig_Node_CheckToString extends Twig_Node ++{ ++ public function __construct(Twig_Node_Expression $expr) ++ { ++ parent::__construct(['expr' => $expr], [], $expr->getTemplateLine(), $expr->getNodeTag()); ++ } ++ ++ public function compile(Twig_Compiler $compiler) ++ { ++ $compiler ++ ->write('$this->extensions[\'Twig_Extension_Sandbox\']->ensureToStringAllowed(') ++ ->subcompile($this->getNode('expr')) ++ ->raw(')') ++ ; ++ } ++} ++ ++class_alias('Twig_Node_CheckToString', 'Twig\Node\CheckToStringNode', false); +diff --git a/lib/Twig/Node/SandboxedPrint.php b/lib/Twig/Node/SandboxedPrint.php +index eb45cb8..aee7d2f 100644 +--- a/lib/Twig/Node/SandboxedPrint.php ++++ b/lib/Twig/Node/SandboxedPrint.php +@@ -17,6 +17,8 @@ + * and if the sandbox is enabled, we need to check that the __toString() + * method is allowed if 'article' is an object. + * ++ * Not used anymore, to be deprecated in 2.x and removed in 3.0 ++ * + * @author Fabien Potencier <fab...@symfony.com> + */ + class Twig_Node_SandboxedPrint extends Twig_Node_Print +diff --git a/lib/Twig/NodeVisitor/Sandbox.php b/lib/Twig/NodeVisitor/Sandbox.php +index 4d41ff6..cdc7ff2 100644 +--- a/lib/Twig/NodeVisitor/Sandbox.php ++++ b/lib/Twig/NodeVisitor/Sandbox.php +@@ -21,6 +21,8 @@ final class Twig_NodeVisitor_Sandbox extends Twig_BaseNodeVisitor + private $filters; + private $functions; + ++ private $needsToStringWrap = false; ++ + protected function doEnterNode(Twig_Node $node, Twig_Environment $env) + { + if ($node instanceof Twig_Node_Module) { +@@ -51,9 +53,28 @@ final class Twig_NodeVisitor_Sandbox extends Twig_BaseNodeVisitor + $this->functions['range'] = $node; + } + +- // wrap print to check __toString() calls + if ($node instanceof Twig_Node_Print) { +- return new Twig_Node_SandboxedPrint($node->getNode('expr'), $node->getTemplateLine(), $node->getNodeTag()); ++ $this->needsToStringWrap = true; ++ $this->wrapNode($node, 'expr'); ++ } ++ ++ if ($node instanceof Twig_Node_Set && !$node->getAttribute('capture')) { ++ $this->needsToStringWrap = true; ++ } ++ ++ // wrap outer nodes that can implicitly call __toString() ++ if ($this->needsToStringWrap) { ++ if ($node instanceof Twig_Node_Expression_Binary_Concat) { ++ $this->wrapNode($node, 'left'); ++ $this->wrapNode($node, 'right'); ++ } ++ if ($node instanceof Twig_Node_Expression_Filter) { ++ $this->wrapNode($node, 'node'); ++ $this->wrapArrayNode($node, 'arguments'); ++ } ++ if ($node instanceof Twig_Node_Expression_Function) { ++ $this->wrapArrayNode($node, 'arguments'); ++ } + } + } + +@@ -66,11 +87,31 @@ final class Twig_NodeVisitor_Sandbox extends Twig_BaseNodeVisitor + $this->inAModule = false; + + $node->setNode('display_start', new Twig_Node([new Twig_Node_CheckSecurity($this->filters, $this->tags, $this->functions), $node->getNode('display_start')])); ++ } elseif ($this->inAModule) { ++ if ($node instanceof Twig_Node_Print || $node instanceof Twig_Node_Set) { ++ $this->needsToStringWrap = false; ++ } + } + + return $node; + } + ++ private function wrapNode(Twig_Node $node, $name) ++ { ++ $expr = $node->getNode($name); ++ if ($expr instanceof Twig_Node_Expression_Name || $expr instanceof Twig_Node_Expression_GetAttr) { ++ $node->setNode($name, new Twig_Node_CheckToString($expr)); ++ } ++ } ++ ++ private function wrapArrayNode(Twig_Node $node, $name) ++ { ++ $args = $node->getNode($name); ++ foreach ($args as $name => $_) { ++ $this->wrapNode($args, $name); ++ } ++ } ++ + public function getPriority() + { + return 0; +diff --git a/src/Node/CheckToStringNode.php b/src/Node/CheckToStringNode.php +new file mode 100644 +index 0000000..d8df055 +--- /dev/null ++++ b/src/Node/CheckToStringNode.php +@@ -0,0 +1,11 @@ ++<?php ++ ++namespace Twig\Node; ++ ++class_exists('Twig_Node_CheckToString'); ++ ++if (\false) { ++ class CheckToStringNode extends \Twig_Node_CheckToString ++ { ++ } ++} +diff --git a/test/Twig/Tests/Extension/SandboxTest.php b/test/Twig/Tests/Extension/SandboxTest.php +index aef39c3..deb4a3d 100644 +--- a/test/Twig/Tests/Extension/SandboxTest.php ++++ b/test/Twig/Tests/Extension/SandboxTest.php +@@ -28,7 +28,6 @@ class Twig_Tests_Extension_SandboxTest extends \PHPUnit\Framework\TestCase + '1_basic3' => '{% if name %}foo{% endif %}', + '1_basic4' => '{{ obj.bar }}', + '1_basic5' => '{{ obj }}', +- '1_basic6' => '{{ arr.obj }}', + '1_basic7' => '{{ cycle(["foo","bar"], 1) }}', + '1_basic8' => '{{ obj.getfoobar }}{{ obj.getFooBar }}', + '1_basic9' => '{{ obj.foobar }}{{ obj.fooBar }}', +@@ -106,11 +105,14 @@ class Twig_Tests_Extension_SandboxTest extends \PHPUnit\Framework\TestCase + } + } + +- public function testSandboxUnallowedToString() ++ /** ++ * @dataProvider getSandboxUnallowedToStringTests ++ */ ++ public function testSandboxUnallowedToString($template) + { +- $twig = $this->getEnvironment(true, [], self::$templates); ++ $twig = $this->getEnvironment(true, [], ['index' => $template], [], ['upper'], ['FooObject' => 'getAnotherFooObject'], [], ['random']); + try { +- $twig->loadTemplate('1_basic5')->render(self::$params); ++ $twig->loadTemplate('index')->render(self::$params); + $this->fail('Sandbox throws a SecurityError exception if an unallowed method (__toString()) is called in the template'); + } catch (Twig_Sandbox_SecurityError $e) { + $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedMethodError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedMethodError'); +@@ -119,17 +121,61 @@ class Twig_Tests_Extension_SandboxTest extends \PHPUnit\Framework\TestCase + } + } + +- public function testSandboxUnallowedToStringArray() ++ public function getSandboxUnallowedToStringTests() + { +- $twig = $this->getEnvironment(true, [], self::$templates); +- try { +- $twig->loadTemplate('1_basic6')->render(self::$params); +- $this->fail('Sandbox throws a SecurityError exception if an unallowed method (__toString()) is called in the template'); +- } catch (Twig_Sandbox_SecurityError $e) { +- $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedMethodError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedMethodError'); +- $this->assertEquals('FooObject', $e->getClassName(), 'Exception should be raised on the "FooObject" class'); +- $this->assertEquals('__tostring', $e->getMethodName(), 'Exception should be raised on the "__toString" method'); +- } ++ return [ ++ 'simple' => ['{{ obj }}'], ++ 'object_from_array' => ['{{ arr.obj }}'], ++ 'object_chain' => ['{{ obj.anotherFooObject }}'], ++ 'filter' => ['{{ obj|upper }}'], ++ 'filter_from_array' => ['{{ arr.obj|upper }}'], ++ 'function' => ['{{ random(obj) }}'], ++ 'function_from_array' => ['{{ random(arr.obj) }}'], ++ 'function_and_filter' => ['{{ random(obj|upper) }}'], ++ 'function_and_filter_from_array' => ['{{ random(arr.obj|upper) }}'], ++ 'object_chain_and_filter' => ['{{ obj.anotherFooObject|upper }}'], ++ 'object_chain_and_function' => ['{{ random(obj.anotherFooObject) }}'], ++ 'concat' => ['{{ obj ~ "" }}'], ++ 'concat_again' => ['{{ "" ~ obj }}'], ++ ]; ++ } ++ ++ /** ++ * @dataProvider getSandboxAllowedToStringTests ++ */ ++ public function testSandboxAllowedToString($template, $output) ++ { ++ $twig = $this->getEnvironment(true, [], ['index' => $template], ['set'], [], ['FooObject' => ['foo', 'getAnotherFooObject']]); ++ $this->assertEquals($output, $twig->load('index')->render(self::$params)); ++ } ++ ++ public function getSandboxAllowedToStringTests() ++ { ++ return [ ++ 'constant_test' => ['{{ obj is constant("PHP_INT_MAX") }}', ''], ++ 'set_object' => ['{% set a = obj.anotherFooObject %}{{ a.foo }}', 'foo'], ++ 'is_defined' => ['{{ obj.anotherFooObject is defined }}', '1'], ++ 'is_null' => ['{{ obj is null }}', ''], ++ 'is_sameas' => ['{{ obj is same as(obj) }}', '1'], ++ 'is_sameas_from_array' => ['{{ arr.obj is same as(arr.obj) }}', '1'], ++ 'is_sameas_from_another_method' => ['{{ obj.anotherFooObject is same as(obj.anotherFooObject) }}', ''], ++ ]; ++ } ++ ++ public function testSandboxAllowMethodToString() ++ { ++ $twig = $this->getEnvironment(true, [], self::$templates, [], [], ['FooObject' => '__toString']); ++ FooObject::reset(); ++ $this->assertEquals('foo', $twig->load('1_basic5')->render(self::$params), 'Sandbox allow some methods'); ++ $this->assertEquals(1, FooObject::$called['__toString'], 'Sandbox only calls method once'); ++ } ++ ++ public function testSandboxAllowMethodToStringDisabled() ++ { ++ $twig = $this->getEnvironment(false, [], self::$templates); ++ FooObject::reset(); ++ $this->assertEquals('foo', $twig->load('1_basic5')->render(self::$params), 'Sandbox allows __toString when sandbox disabled'); ++ $this->assertEquals(1, FooObject::$called['__toString'], 'Sandbox only calls method once'); + } + + public function testSandboxUnallowedFunction() +@@ -164,22 +210,6 @@ class Twig_Tests_Extension_SandboxTest extends \PHPUnit\Framework\TestCase + $this->assertEquals(1, FooObject::$called['foo'], 'Sandbox only calls method once'); + } + +- public function testSandboxAllowMethodToString() +- { +- $twig = $this->getEnvironment(true, [], self::$templates, [], [], ['FooObject' => '__toString']); +- FooObject::reset(); +- $this->assertEquals('foo', $twig->loadTemplate('1_basic5')->render(self::$params), 'Sandbox allow some methods'); +- $this->assertEquals(1, FooObject::$called['__toString'], 'Sandbox only calls method once'); +- } +- +- public function testSandboxAllowMethodToStringDisabled() +- { +- $twig = $this->getEnvironment(false, [], self::$templates); +- FooObject::reset(); +- $this->assertEquals('foo', $twig->loadTemplate('1_basic5')->render(self::$params), 'Sandbox allows __toString when sandbox disabled'); +- $this->assertEquals(1, FooObject::$called['__toString'], 'Sandbox only calls method once'); +- } +- + public function testSandboxAllowFilter() + { + $twig = $this->getEnvironment(true, [], self::$templates, [], ['upper']); +@@ -319,4 +349,9 @@ class FooObject + + return 'foobar'; + } ++ ++ public function getAnotherFooObject() ++ { ++ return new self(); ++ } + } +diff --git a/test/Twig/Tests/Node/SandboxedPrintTest.php b/test/Twig/Tests/Node/SandboxedPrintTest.php +deleted file mode 100644 +index 269a461..0000000 +--- a/test/Twig/Tests/Node/SandboxedPrintTest.php ++++ /dev/null +@@ -1,33 +0,0 @@ +-<?php +- +-/* +- * This file is part of Twig. +- * +- * (c) Fabien Potencier +- * +- * For the full copyright and license information, please view the LICENSE +- * file that was distributed with this source code. +- */ +- +-class Twig_Tests_Node_SandboxedPrintTest extends Twig_Test_NodeTestCase +-{ +- public function testConstructor() +- { +- $node = new Twig_Node_SandboxedPrint($expr = new Twig_Node_Expression_Constant('foo', 1), 1); +- +- $this->assertEquals($expr, $node->getNode('expr')); +- } +- +- public function getTests() +- { +- $tests = []; +- +- $tests[] = [new Twig_Node_SandboxedPrint(new Twig_Node_Expression_Constant('foo', 1), 1), <<<EOF +-// line 1 +-echo \$this->extensions['Twig_Extension_Sandbox']->ensureToStringAllowed("foo"); +-EOF +- ]; +- +- return $tests; +- } +-} diff --git a/debian/patches/series b/debian/patches/series new file mode 100644 index 00000000..3dd5ef20 --- /dev/null +++ b/debian/patches/series @@ -0,0 +1 @@ +0001-Fix-security-issue-in-the-sandbox.patch
signature.asc
Description: PGP signature
