diff --git a/src/Template.php b/src/Template.php index 032efe32699..a233516ce38 100644 --- a/src/Template.php +++ b/src/Template.php @@ -15,6 +15,8 @@ use Twig\Error\Error; use Twig\Error\LoaderError; use Twig\Error\RuntimeError; +use Twig\Extension\SandboxExtension; +use Twig\Sandbox\SecurityError; /** * Default base class for compiled templates. @@ -526,6 +528,7 @@ final protected function getContext($context, $item, $ignoreStrictCheck = false) * @return mixed The attribute value, or a Boolean when $isDefinedTest is true, or null when the attribute is not set and $ignoreStrictCheck is true * * @throws RuntimeError if the attribute does not exist and Twig is running in strict mode and $isDefinedTest is false + * @throws SecurityError if the attribute is not allowed * * @internal */ @@ -601,6 +604,7 @@ protected function getAttribute($object, $item, array $arguments = [], $type = s } // object property + $propertySandboxException = null; if (self::METHOD_CALL !== $type && !$object instanceof self) { // \Twig\Template does not have public properties, and we don't want to allow access to internal ones if (isset($object->$item) || \array_key_exists((string) $item, (array) $object)) { if ($isDefinedTest) { @@ -608,10 +612,15 @@ protected function getAttribute($object, $item, array $arguments = [], $type = s } if ($this->env->hasExtension('\Twig\Extension\SandboxExtension')) { - $this->env->getExtension('\Twig\Extension\SandboxExtension')->checkPropertyAllowed($object, $item); + try { + $this->env->getExtension('\Twig\Extension\SandboxExtension')->checkPropertyAllowed($object, $item); + } catch (SecurityError $propertySandboxException) { + } } - return $object->$item; + if (null === $propertySandboxException) { + return $object->$item; + } } } @@ -678,6 +687,10 @@ protected function getAttribute($object, $item, array $arguments = [], $type = s return false; } + if (null !== $propertySandboxException) { + throw $propertySandboxException; + } + if ($ignoreStrictCheck || !$this->env->isStrictVariables()) { return; } @@ -690,7 +703,15 @@ protected function getAttribute($object, $item, array $arguments = [], $type = s } if ($this->env->hasExtension('\Twig\Extension\SandboxExtension')) { - $this->env->getExtension('\Twig\Extension\SandboxExtension')->checkMethodAllowed($object, $method); + try { + $this->env->getExtension(SandboxExtension::class)->checkMethodAllowed($object, $call ? '__call' : $method); + } catch (SecurityError $e) { + if ($call && null !== $propertySandboxException) { + throw $propertySandboxException; + } + + throw $e; + } } // Some objects throw exceptions when they have __call, and the method we try @@ -702,6 +723,10 @@ protected function getAttribute($object, $item, array $arguments = [], $type = s $ret = \call_user_func_array([$object, $method], $arguments); } } catch (\BadMethodCallException $e) { + if ($call && null !== $propertySandboxException) { + throw $propertySandboxException; + } + if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) { return; } diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index a1a989f70d7..cf2ec3bc7b5 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -41,6 +41,9 @@ protected function setUp(): void '1_basic7' => '{{ cycle(["foo","bar"], 1) }}', '1_basic8' => '{{ obj.getfoobar }}{{ obj.getFooBar }}', '1_basic9' => '{{ obj.foobar }}{{ obj.fooBar }}', + '1_basic10' => '{{ obj.baz }}', + '1_basic11' => '{{ obj.xyz }}', + '1_basic12' => '{{ obj.bac }}', '1_basic' => '{% if obj.foo %}{{ obj.foo|upper }}{% endif %}', '1_layout' => '{% block content %}{% endblock %}', '1_child' => "{% extends \"1_layout\" %}\n{% block content %}\n{{ \"a\"|json_encode }}\n{% endblock %}", @@ -326,6 +329,35 @@ public function testSandboxAllowFunctionsCaseInsensitive() } } + public function testSandboxRunGetterInsteadOfUnallowedProperty() + { + $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array(FooObject::class => 'getBaz')); + FooObject::reset(); + $this->assertEquals('baz', $twig->load('1_basic10')->render(self::$params), 'Sandbox allow some methods'); + $this->assertEquals(1, FooObject::$called['getBaz'], 'Sandbox only calls method getBaz'); + } + + public function testSandboxRunCallInsteadOfUnallowedProperty() + { + $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array(FooObject::class => '__call')); + FooObject::reset(); + $this->assertEquals('xyz', $twig->load('1_basic11')->render(self::$params), 'Sandbox allow a method (__call())'); + $this->assertEquals(1, FooObject::$called['__call'], 'Sandbox only calls method __call'); + } + + public function testSandboxRunCallInsteadOfUnallowedPropertyAndThrowException() + { + $twig = $this->getEnvironment(true, array(), self::$templates, [], [], [FooObject::class => '__call']); + FooObject::reset(); + try { + $twig->load('1_basic12')->render(self::$params); + $this->fail('Sandbox throws a SecurityError exception if an unallowed property is called in the template through a method (__call)'); + } catch (SecurityError $e) { + $this->assertEquals(sprintf('Calling "bac" property on a "%s" object is not allowed.', FooObject::class), $e->getRawMessage()); + $this->assertEquals(1, FooObject::$called['__call'], 'Sandbox only calls method __call'); + } + } + public function testSandboxLocallySetForAnInclude() { self::$templates = [ @@ -418,13 +450,17 @@ protected function getEnvironment($sandboxed, $options, $templates, $tags = [], class FooObject { - public static $called = ['__toString' => 0, 'foo' => 0, 'getFooBar' => 0]; + public static $called = ['__call' => 0, '__toString' => 0, 'foo' => 0, 'getBaz' => 0, 'getFooBar' => 0]; public $bar = 'bar'; + public $baz = 'baz'; + + public $bac = 'bac'; + public static function reset() { - self::$called = ['__toString' => 0, 'foo' => 0, 'getFooBar' => 0]; + self::$called = ['__call' => 0, '__toString' => 0, 'foo' => 0, 'getBaz' => 0, 'getFooBar' => 0]; } public function __toString() @@ -452,4 +488,20 @@ public function getAnotherFooObject() { return new self(); } + + public function getBaz() + { + ++self::$called['getBaz']; + return $this->baz; + } + + public function __call($name, $arguments) + { + ++self::$called['__call']; + if ('bac' === strtolower($name)) { + throw new \BadMethodCallException('bac() method is not allowed'); + } + + return $name; + } }