feature #2176 Improved Sandbox error handling (kitbs)

This PR was squashed before being merged into the 1.x branch (closes #2176). Discussion ---------- Improved Sandbox error handling On the model of the SecurityNotAllowedFilterError, SecurityNotAllowedFunctionError and SecurityNotAllowedTagError, I added SecurityNotAllowedMethodError and SecurityNotAllowedPropertyError to allow the user to retrieve both the $className and $methodName/$propertyName from the exception. Commits ------- e1c980f Improved Sandbox error handling
twigphp · Oct 12, 2016 · 3879038 · 3879038
2 parents 614ade9 + e1c980f
commit 3879038
Show file tree

Hide file tree

Showing 5 changed files with 146 additions and 3 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,6 @@
 * 1.26.2 (2016-XX-XX)
 
- * n/a
+ * improved debugging with Twig_Sandbox_SecurityError exceptions for disallowed methods and properties
 
 * 1.26.1 (2016-10-05)
 

diff --git a/lib/Twig/Sandbox/SecurityNotAllowedMethodError.php b/lib/Twig/Sandbox/SecurityNotAllowedMethodError.php
@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) 2009 Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+/**
+ * Exception thrown when a not allowed class method is used in a template.
+ *
+ * @author Kit Burton-Senior <mail@kitbs.com>
+ */
+class Twig_Sandbox_SecurityNotAllowedMethodError extends Twig_Sandbox_SecurityError
+{
+    private $className;
+    private $methodName;
+
+    public function __construct($message, $className, $methodName, $lineno = -1, $filename = null, Exception $previous = null)
+    {
+        parent::__construct($message, $lineno, $filename, $previous);
+        $this->className = $className;
+        $this->methodName = $methodName;
+    }
+
+    public function getClassName()
+    {
+        return $this->className;
+    }
+
+    public function getMethodName()
+    {
+        return $this->methodName;
+    }
+}
diff --git a/lib/Twig/Sandbox/SecurityNotAllowedPropertyError.php b/lib/Twig/Sandbox/SecurityNotAllowedPropertyError.php
@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) 2009 Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+/**
+ * Exception thrown when a not allowed class property is used in a template.
+ *
+ * @author Kit Burton-Senior <mail@kitbs.com>
+ */
+class Twig_Sandbox_SecurityNotAllowedPropertyError extends Twig_Sandbox_SecurityError
+{
+    private $className;
+    private $propertyName;
+
+    public function __construct($message, $className, $propertyName, $lineno = -1, $filename = null, Exception $previous = null)
+    {
+        parent::__construct($message, $lineno, $filename, $previous);
+        $this->className = $className;
+        $this->propertyName = $propertyName;
+    }
+
+    public function getClassName()
+    {
+        return $this->className;
+    }
+
+    public function getPropertyName()
+    {
+        return $this->propertyName;
+    }
+}
diff --git a/lib/Twig/Sandbox/SecurityPolicy.php b/lib/Twig/Sandbox/SecurityPolicy.php
@@ -97,7 +97,8 @@ public function checkMethodAllowed($obj, $method)
         }
 
         if (!$allowed) {
-            throw new Twig_Sandbox_SecurityError(sprintf('Calling "%s" method on a "%s" object is not allowed.', $method, get_class($obj)));
+            $class = get_class($obj);
+            throw new Twig_Sandbox_SecurityNotAllowedMethodError(sprintf('Calling "%s" method on a "%s" object is not allowed.', $method, $class), $class, $method);
         }
     }
 
@@ -113,7 +114,8 @@ public function checkPropertyAllowed($obj, $property)
         }
 
         if (!$allowed) {
-            throw new Twig_Sandbox_SecurityError(sprintf('Calling "%s" property on a "%s" object is not allowed.', $property, get_class($obj)));
+            $class = get_class($obj);
+            throw new Twig_Sandbox_SecurityNotAllowedPropertyError(sprintf('Calling "%s" property on a "%s" object is not allowed.', $property, $class), $class, $property);
         }
     }
 }
diff --git a/test/Twig/Tests/Extension/SandboxTest.php b/test/Twig/Tests/Extension/SandboxTest.php
@@ -51,83 +51,146 @@ public function testSandboxGloballySet()
     {
         $twig = $this->getEnvironment(false, array(), self::$templates);
         $this->assertEquals('FOO', $twig->loadTemplate('1_basic')->render(self::$params), 'Sandbox does nothing if it is disabled globally');
+    }
 
+    public function testSandboxUnallowedMethodAccessor()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic1')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception if an unallowed method is called');
         } 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('foo', $e->getMethodName(), 'Exception should be raised on the "foo" method');
         }
+    }
 
+    public function testSandboxUnallowedFilter()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic2')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception if an unallowed filter is called');
         } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedFilterError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedFilterError');
+            $this->assertEquals('upper', $e->getFilterName(), 'Exception should be raised on the "upper" filter');
         }
+    }
 
+    public function testSandboxUnallowedTag()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic3')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception if an unallowed tag is used in the template');
         } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedTagError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedTagError');
+            $this->assertEquals('if', $e->getTagName(), 'Exception should be raised on the "if" tag');
         }
+    }
 
+    public function testSandboxUnallowedProperty()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic4')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception if an unallowed property is called in the template');
         } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedPropertyError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedPropertyError');
+            $this->assertEquals('FooObject', $e->getClassName(), 'Exception should be raised on the "FooObject" class');
+            $this->assertEquals('bar', $e->getPropertyName(), 'Exception should be raised on the "bar" property');
         }
+    }
 
+    public function testSandboxUnallowedToString()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic5')->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');
         }
+    }
 
+    public function testSandboxUnallowedToStringArray()
+    {
         $twig = $this->getEnvironment(true, array(), 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');
         }
+    }
 
+    public function testSandboxUnallowedFunction()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates);
         try {
             $twig->loadTemplate('1_basic7')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception if an unallowed function is called in the template');
         } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedFunctionError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedFunctionError');
+            $this->assertEquals('cycle', $e->getFunctionName(), 'Exception should be raised on the "cycle" function');
         }
+    }
 
+    public function testSandboxAllowMethodFoo()
+    {
         $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');
         $this->assertEquals(1, FooObject::$called['foo'], 'Sandbox only calls method once');
+    }
 
+    public function testSandboxAllowMethodToString()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('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, array(), 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, array(), self::$templates, array(), array('upper'));
         $this->assertEquals('FABIEN', $twig->loadTemplate('1_basic2')->render(self::$params), 'Sandbox allow some filters');
+    }
 
+    public function testSandboxAllowTag()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates, array('if'));
         $this->assertEquals('foo', $twig->loadTemplate('1_basic3')->render(self::$params), 'Sandbox allow some tags');
+    }
 
+    public function testSandboxAllowProperty()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array(), array('FooObject' => 'bar'));
         $this->assertEquals('bar', $twig->loadTemplate('1_basic4')->render(self::$params), 'Sandbox allow some properties');
+    }
 
+    public function testSandboxAllowFunction()
+    {
         $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array(), array(), array('cycle'));
         $this->assertEquals('bar', $twig->loadTemplate('1_basic7')->render(self::$params), 'Sandbox allow some functions');
+    }
 
+    public function testSandboxAllowFunctionsCaseInsensitive()
+    {
         foreach (array('getfoobar', 'getFoobar', 'getFooBar') as $name) {
             $twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => $name));
             FooObject::reset();
@@ -158,6 +221,8 @@ public function testSandboxLocallySetForAnInclude()
             $twig->loadTemplate('3_basic')->render(self::$params);
             $this->fail('Sandbox throws a SecurityError exception when the included file is sandboxed');
         } catch (Twig_Sandbox_SecurityError $e) {
+            $this->assertInstanceOf('Twig_Sandbox_SecurityNotAllowedTagError', $e, 'Exception should be an instance of Twig_Sandbox_SecurityNotAllowedTagError');
+            $this->assertEquals('sandbox', $e->getTagName());
         }
     }