Your message dated Sat, 23 Mar 2019 21:36:13 +0000 with message-id <20190323213613.ga3...@powdarrmonkey.net> and subject line Re: Bug#925319: unblock: twig/2.6.2-2 has caused the Debian Bug report #925319, regarding unblock: twig/2.6.2-2 to be marked as done.
This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the Bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact ow...@bugs.debian.org immediately.) -- 925319: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925319 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
--- Begin Message ---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 Daviddiff --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.patchsignature.asc
Description: PGP signature
--- End Message ---
--- Begin Message ---On Fri, Mar 22, 2019 at 02:22:39PM -1000, David Prévot wrote: > 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 Unblocked; thanks. -- Jonathan Wiltshire j...@debian.org Debian Developer http://people.debian.org/~jmw 4096R: 0xD3524C51 / 0A55 B7C5 1223 3942 86EC 74C3 5394 479D D352 4C51
--- End Message ---