Package: release.debian.org Severity: normal User: release.debian.org@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 <taffit@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?= <david@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 <fabien@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 <fabien@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
Attachment:
signature.asc
Description: PGP signature