Fixed sandbox for methods

Author: hason

lib/Twig/Template.php

@@ -523,18 +523,21 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
         }
 
         // object property
-        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, $object)) {
-                if ($isDefinedTest) {
-                    return true;
-                }
+        try {
+            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, $object)) {
+                    if ($isDefinedTest) {
+                        return true;
+                    }
 
-                if ($this->env->hasExtension('sandbox')) {
-                    $this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
-                }
+                    if ($this->env->hasExtension('sandbox')) {
+                        $this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
+                    }
 
-                return $object->$item;
+                    return $object->$item;
+                }
             }
+        } catch (Twig_Sandbox_SecurityError $propertySandboxException) {
         }
 
         $class = get_class($object);
@@ -573,6 +576,10 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
             $method = (string) $item;
             $call = true;
         } else {
+            if (isset($propertySandboxException)) {
+                throw $propertySandboxException;
+            }
+
             if ($isDefinedTest) {
                 return false;
             }
@@ -589,14 +596,18 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
         }
 
         if ($this->env->hasExtension('sandbox')) {
-            $this->env->getExtension('sandbox')->checkMethodAllowed($object, $method);
+            $this->env->getExtension('sandbox')->checkMethodAllowed($object, $call ? '__call' : $method);
         }
 
         // Some objects throw exceptions when they have __call, and the method we try
         // to call is not supported. If ignoreStrictCheck is true, we should return null.
         try {
             $ret = call_user_func_array(array($object, $method), $arguments);
         } catch (BadMethodCallException $e) {
+            if ($call && isset($propertySandboxException)) {
+                throw $propertySandboxException;
+            }
+
             if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
                 return;
             }

test/Twig/Tests/Extension/SandboxTest.php

@@ -31,6 +31,9 @@ protected function setUp()
             '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.bac }}',
+            '1_basic12' => '{{ obj.xyz }}',
             '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 %}",
@@ -101,6 +104,14 @@ public function testSandboxGloballySet()
         } catch (Twig_Sandbox_SecurityError $e) {
         }
 
+        $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => '__call'));
+        try {
+            $twig->loadTemplate('1_basic11')->render(self::$params);
+            $this->fail('Sandbox throws a SecurityError exception if an unallowed property is called in the template through a method (__call)');
+        } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertEquals('Calling "bac" property on a "FooObject" object is not allowed.', $e->getRawMessage());
+        }
+
         $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => 'foo'));
         FooObject::reset();
         $this->assertEquals('foo', $twig->loadTemplate('1_basic1')->render(self::$params), 'Sandbox allow some methods');
@@ -136,6 +147,12 @@ public function testSandboxGloballySet()
 
             $this->assertEquals('foobarfoobar', $twig->loadTemplate('1_basic9')->render(self::$params), 'Sandbox allow methods via shortcut names (ie. without get/set)');
         }
+
+        $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => 'getBaz'));
+        $this->assertEquals('baz', $twig->loadTemplate('1_basic10')->render(self::$params), 'Sandbox allow some methods');
+
+        $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => '__call'));
+        $this->assertEquals('xyz', $twig->loadTemplate('1_basic12')->render(self::$params), 'Sandbox allow a method (__call())');
     }
 
     public function testSandboxLocallySetForAnInclude()
@@ -192,6 +209,10 @@ class FooObject
 
     public $bar = 'bar';
 
+    public $baz = 'baz';
+
+    public $bac = 'bac';
+
     public static function reset()
     {
         self::$called = array('__toString' => 0, 'foo' => 0, 'getFooBar' => 0);
@@ -217,4 +238,18 @@ public function getFooBar()
 
         return 'foobar';
     }
+
+    public function getBaz()
+    {
+        return $this->baz;
+    }
+
+    public function __call($name, $arguments)
+    {
+        if ('bac' === strtolower($name)) {
+            throw new BadMethodCallException;
+        }
+
+        return $name;
+    }
 }