From 58efc421dc4fc3c15c7323babcf7506f6d60787c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Martin=20Haso=C5=88?= Date: Fri, 10 Oct 2014 15:36:50 +0200 Subject: [PATCH] Improved error reporting in a sandboxed template --- lib/Twig/Node/SandboxedModule.php | 42 ++++++++++++++- lib/Twig/NodeVisitor/Sandbox.php | 14 +++--- lib/Twig/Sandbox/SecurityNotAllowedFilterError.php | 31 +++++++++++ .../Sandbox/SecurityNotAllowedFunctionError.php | 31 +++++++++++ lib/Twig/Sandbox/SecurityNotAllowedTagError.php | 31 +++++++++++ lib/Twig/Sandbox/SecurityPolicy.php | 6 +- test/Twig/Tests/Extension/SandboxTest.php | 4 +- .../Tests/Fixtures/functions/include/sandbox.test | 5 ++- test/Twig/Tests/Node/SandboxedModuleTest.php | 56 ++++++++++++++++---- 9 files changed, 194 insertions(+), 26 deletions(-) create mode 100644 lib/Twig/Sandbox/SecurityNotAllowedFilterError.php create mode 100644 lib/Twig/Sandbox/SecurityNotAllowedFunctionError.php create mode 100644 lib/Twig/Sandbox/SecurityNotAllowedTagError.php diff --git a/lib/Twig/Node/SandboxedModule.php b/lib/Twig/Node/SandboxedModule.php index 0de4503..410332c 100644 --- a/lib/Twig/Node/SandboxedModule.php +++ b/lib/Twig/Node/SandboxedModule.php @@ -43,17 +43,53 @@ class Twig_Node_SandboxedModule extends Twig_Node_Module { parent::compileDisplayFooter($compiler); + $tags = $filters = $functions = array(); + foreach (array('tags', 'filters', 'functions') as $type) { + foreach ($this->{'used'.ucfirst($type)} as $name => $node) { + if ($node instanceof Twig_Node) { + ${$type}[$name] = $node->getLine(); + } else { + ${$type}[$node] = null; + } + } + } + $compiler ->write("protected function checkSecurity()\n", "{\n") ->indent() + ->write("\$tags = ")->repr(array_filter($tags))->raw(";\n") + ->write("\$filters = ")->repr(array_filter($filters))->raw(";\n") + ->write("\$functions = ")->repr(array_filter($functions))->raw(";\n\n") + ->write("try {\n") + ->indent() ->write("\$this->env->getExtension('sandbox')->checkSecurity(\n") ->indent() - ->write(!$this->usedTags ? "array(),\n" : "array('".implode('\', \'', $this->usedTags)."'),\n") - ->write(!$this->usedFilters ? "array(),\n" : "array('".implode('\', \'', $this->usedFilters)."'),\n") - ->write(!$this->usedFunctions ? "array()\n" : "array('".implode('\', \'', $this->usedFunctions)."')\n") + ->write(!$tags ? "array(),\n" : "array('".implode("', '", array_keys($tags))."'),\n") + ->write(!$filters ? "array(),\n" : "array('".implode("', '", array_keys($filters))."'),\n") + ->write(!$functions ? "array()\n" : "array('".implode("', '", array_keys($functions))."')\n") ->outdent() ->write(");\n") ->outdent() + ->write("} catch (Twig_Sandbox_SecurityError \$e) {\n") + ->indent() + ->write("\$e->setTemplateFile(\$this->getTemplateName());\n\n") + ->write("if (\$e instanceof Twig_Sandbox_SecurityNotAllowedTagError && isset(\$tags[\$e->getTagName()])) {\n") + ->indent() + ->write("\$e->setTemplateLine(\$tags[\$e->getTagName()]);\n") + ->outdent() + ->write("} elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFilterError && isset(\$filters[\$e->getFilterName()])) {\n") + ->indent() + ->write("\$e->setTemplateLine(\$filters[\$e->getFilterName()]);\n") + ->outdent() + ->write("} elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFunctionError && isset(\$functions[\$e->getFunctionName()])) {\n") + ->indent() + ->write("\$e->setTemplateLine(\$functions[\$e->getFunctionName()]);\n") + ->outdent() + ->write("}\n\n") + ->write("throw \$e;\n") + ->outdent() + ->write("}\n") + ->outdent() ->write("}\n\n") ; } diff --git a/lib/Twig/NodeVisitor/Sandbox.php b/lib/Twig/NodeVisitor/Sandbox.php index fb27045..e5e3ff6 100644 --- a/lib/Twig/NodeVisitor/Sandbox.php +++ b/lib/Twig/NodeVisitor/Sandbox.php @@ -40,18 +40,18 @@ class Twig_NodeVisitor_Sandbox implements Twig_NodeVisitorInterface return $node; } elseif ($this->inAModule) { // look for tags - if ($node->getNodeTag()) { - $this->tags[] = $node->getNodeTag(); + if ($node->getNodeTag() && !isset($this->tags[$node->getNodeTag()])) { + $this->tags[$node->getNodeTag()] = $node; } // look for filters - if ($node instanceof Twig_Node_Expression_Filter) { - $this->filters[] = $node->getNode('filter')->getAttribute('value'); + if ($node instanceof Twig_Node_Expression_Filter && !isset($this->filters[$node->getNode('filter')->getAttribute('value')])) { + $this->filters[$node->getNode('filter')->getAttribute('value')] = $node; } // look for functions - if ($node instanceof Twig_Node_Expression_Function) { - $this->functions[] = $node->getAttribute('name'); + if ($node instanceof Twig_Node_Expression_Function && !isset($this->functions[$node->getAttribute('name')])) { + $this->functions[$node->getAttribute('name')] = $node; } // wrap print to check __toString() calls @@ -76,7 +76,7 @@ class Twig_NodeVisitor_Sandbox implements Twig_NodeVisitorInterface if ($node instanceof Twig_Node_Module) { $this->inAModule = false; - return new Twig_Node_SandboxedModule($node, array_unique($this->filters), array_unique($this->tags), array_unique($this->functions)); + return new Twig_Node_SandboxedModule($node, $this->filters, $this->tags, $this->functions); } return $node; diff --git a/lib/Twig/Sandbox/SecurityNotAllowedFilterError.php b/lib/Twig/Sandbox/SecurityNotAllowedFilterError.php new file mode 100644 index 0000000..99faba9 --- /dev/null +++ b/lib/Twig/Sandbox/SecurityNotAllowedFilterError.php @@ -0,0 +1,31 @@ + + */ +class Twig_Sandbox_SecurityNotAllowedFilterError extends Twig_Sandbox_SecurityError +{ + private $filterName; + + public function __construct($message, $functionName, $lineno = -1, $filename = null, Exception $previous = null) + { + parent::__construct($message, $lineno, $filename, $previous); + $this->filterName = $functionName; + } + + public function getFilterName() + { + return $this->filterName; + } +} diff --git a/lib/Twig/Sandbox/SecurityNotAllowedFunctionError.php b/lib/Twig/Sandbox/SecurityNotAllowedFunctionError.php new file mode 100644 index 0000000..05cf488 --- /dev/null +++ b/lib/Twig/Sandbox/SecurityNotAllowedFunctionError.php @@ -0,0 +1,31 @@ + + */ +class Twig_Sandbox_SecurityNotAllowedFunctionError extends Twig_Sandbox_SecurityError +{ + private $functionName; + + public function __construct($message, $functionName, $lineno = -1, $filename = null, Exception $previous = null) + { + parent::__construct($message, $lineno, $filename, $previous); + $this->functionName = $functionName; + } + + public function getFunctionName() + { + return $this->functionName; + } +} diff --git a/lib/Twig/Sandbox/SecurityNotAllowedTagError.php b/lib/Twig/Sandbox/SecurityNotAllowedTagError.php new file mode 100644 index 0000000..b3bb5e8 --- /dev/null +++ b/lib/Twig/Sandbox/SecurityNotAllowedTagError.php @@ -0,0 +1,31 @@ + + */ +class Twig_Sandbox_SecurityNotAllowedTagError extends Twig_Sandbox_SecurityError +{ + private $tagName; + + public function __construct($message, $tagName, $lineno = -1, $filename = null, Exception $previous = null) + { + parent::__construct($message, $lineno, $filename, $previous); + $this->tagName = $tagName; + } + + public function getTagName() + { + return $this->tagName; + } +} diff --git a/lib/Twig/Sandbox/SecurityPolicy.php b/lib/Twig/Sandbox/SecurityPolicy.php index 66ee233..c4dd03d 100644 --- a/lib/Twig/Sandbox/SecurityPolicy.php +++ b/lib/Twig/Sandbox/SecurityPolicy.php @@ -63,19 +63,19 @@ class Twig_Sandbox_SecurityPolicy implements Twig_Sandbox_SecurityPolicyInterfac { foreach ($tags as $tag) { if (!in_array($tag, $this->allowedTags)) { - throw new Twig_Sandbox_SecurityError(sprintf('Tag "%s" is not allowed.', $tag)); + throw new Twig_Sandbox_SecurityNotAllowedTagError(sprintf('Tag "%s" is not allowed.', $tag), $tag); } } foreach ($filters as $filter) { if (!in_array($filter, $this->allowedFilters)) { - throw new Twig_Sandbox_SecurityError(sprintf('Filter "%s" is not allowed.', $filter)); + throw new Twig_Sandbox_SecurityNotAllowedFilterError(sprintf('Filter "%s" is not allowed.', $filter), $filter); } } foreach ($functions as $function) { if (!in_array($function, $this->allowedFunctions)) { - throw new Twig_Sandbox_SecurityError(sprintf('Function "%s" is not allowed.', $function)); + throw new Twig_Sandbox_SecurityNotAllowedFunctionError(sprintf('Function "%s" is not allowed.', $function), $function); } } } diff --git a/test/Twig/Tests/Extension/SandboxTest.php b/test/Twig/Tests/Extension/SandboxTest.php index 1022338..fee35a0 100644 --- a/test/Twig/Tests/Extension/SandboxTest.php +++ b/test/Twig/Tests/Extension/SandboxTest.php @@ -33,13 +33,13 @@ class Twig_Tests_Extension_SandboxTest extends PHPUnit_Framework_TestCase '1_basic9' => '{{ obj.foobar }}{{ obj.fooBar }}', '1_basic' => '{% if obj.foo %}{{ obj.foo|upper }}{% endif %}', '1_layout' => '{% block content %}{% endblock %}', - '1_child' => '{% extends "1_layout" %}{% block content %}{{ "a"|json_encode }}{% endblock %}', + '1_child' => "{% extends \"1_layout\" %}\n{% block content %}\n{{ \"a\"|json_encode }}\n{% endblock %}", ); } /** * @expectedException Twig_Sandbox_SecurityError - * @expectedExceptionMessage Filter "json_encode" is not allowed in "1_child". + * @expectedExceptionMessage Filter "json_encode" is not allowed in "1_child" at line 3. */ public function testSandboxWithInheritance() { diff --git a/test/Twig/Tests/Fixtures/functions/include/sandbox.test b/test/Twig/Tests/Fixtures/functions/include/sandbox.test index 788a2ab..7b9ccac 100644 --- a/test/Twig/Tests/Fixtures/functions/include/sandbox.test +++ b/test/Twig/Tests/Fixtures/functions/include/sandbox.test @@ -3,8 +3,11 @@ --TEMPLATE-- {{ include("foo.twig", sandboxed = true) }} --TEMPLATE(foo.twig)-- + + +{{ foo|e }} {{ foo|e }} --DATA-- return array() --EXCEPTION-- -Twig_Sandbox_SecurityError: Filter "e" is not allowed in "index.twig" at line 2. +Twig_Sandbox_SecurityNotAllowedFilterError: Filter "e" is not allowed in "foo.twig" at line 4. diff --git a/test/Twig/Tests/Node/SandboxedModuleTest.php b/test/Twig/Tests/Node/SandboxedModuleTest.php index 421f210..bb9ffb7 100644 --- a/test/Twig/Tests/Node/SandboxedModuleTest.php +++ b/test/Twig/Tests/Node/SandboxedModuleTest.php @@ -84,11 +84,29 @@ class __TwigTemplate_a2bfbf7dd6ab85666684fe9297f69363a3fc2046d90f22a317d380c1863 protected function checkSecurity() { - \$this->env->getExtension('sandbox')->checkSecurity( - array('upper'), - array('for'), - array('cycle') - ); + \$tags = array(); + \$filters = array(); + \$functions = array(); + + try { + \$this->env->getExtension('sandbox')->checkSecurity( + array('upper'), + array('for'), + array('cycle') + ); + } catch (Twig_Sandbox_SecurityError \$e) { + \$e->setTemplateFile(\$this->getTemplateName()); + + if (\$e instanceof Twig_Sandbox_SecurityNotAllowedTagError && isset(\$tags[\$e->getTagName()])) { + \$e->setTemplateLine(\$tags[\$e->getTagName()]); + } elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFilterError && isset(\$filters[\$e->getFilterName()])) { + \$e->setTemplateLine(\$filters[\$e->getFilterName()]); + } elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFunctionError && isset(\$functions[\$e->getFunctionName()])) { + \$e->setTemplateLine(\$functions[\$e->getFunctionName()]); + } + + throw \$e; + } } public function getTemplateName() @@ -143,11 +161,29 @@ class __TwigTemplate_a2bfbf7dd6ab85666684fe9297f69363a3fc2046d90f22a317d380c1863 protected function checkSecurity() { - \$this->env->getExtension('sandbox')->checkSecurity( - array('upper'), - array('for'), - array('cycle') - ); + \$tags = array(); + \$filters = array(); + \$functions = array(); + + try { + \$this->env->getExtension('sandbox')->checkSecurity( + array('upper'), + array('for'), + array('cycle') + ); + } catch (Twig_Sandbox_SecurityError \$e) { + \$e->setTemplateFile(\$this->getTemplateName()); + + if (\$e instanceof Twig_Sandbox_SecurityNotAllowedTagError && isset(\$tags[\$e->getTagName()])) { + \$e->setTemplateLine(\$tags[\$e->getTagName()]); + } elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFilterError && isset(\$filters[\$e->getFilterName()])) { + \$e->setTemplateLine(\$filters[\$e->getFilterName()]); + } elseif (\$e instanceof Twig_Sandbox_SecurityNotAllowedFunctionError && isset(\$functions[\$e->getFunctionName()])) { + \$e->setTemplateLine(\$functions[\$e->getFunctionName()]); + } + + throw \$e; + } } public function getTemplateName() -- 1.7.2.5