Skip to content

Commit

Permalink
Fixed sandbox for methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@hason
hason committed Jan 20, 2016
1 parent eb6e05e commit e20380f
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 23 deletions.
68 changes: 48 additions & 20 deletions ext/twig/twig.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ PHP_FUNCTION(twig_template_get_attributes)
char *class_name = NULL;
zval *tmp_class;
char *type_name;
zval *propertySandboxException;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ozz|asbb", &template, &object, &zitem, &arguments, &type, &type_len, &isDefinedTest, &ignoreStrictCheck) == FAILURE) {
return;
Expand Down Expand Up @@ -917,10 +918,15 @@ PHP_FUNCTION(twig_template_get_attributes)
}
if ($this->env->hasExtension('sandbox')) {
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
try {
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
} catch (Twig_Sandbox_SecurityError $propertySandboxException) {
}
}
return $object->$item;
if (!isset($propertySandboxException)) {
return $object->$item;
}
}
}
*/
Expand All @@ -938,14 +944,17 @@ PHP_FUNCTION(twig_template_get_attributes)
if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkPropertyAllowed", object, zitem TSRMLS_CC);
}
if (EG(exception)) {
if (EG(exception) && TWIG_INSTANCE_OF_USERLAND(EG(exception), "Twig_Sandbox_SecurityError" TSRMLS_CC)) {
propertySandboxException = EG(exception);
EG(exception) = NULL;
} else if (EG(exception)) {
efree(item);
return;
} else {
ret = TWIG_PROPERTY(object, zitem TSRMLS_CC);
efree(item);
RETURN_ZVAL(ret, 1, 0);
}

ret = TWIG_PROPERTY(object, zitem TSRMLS_CC);
efree(item);
RETURN_ZVAL(ret, 1, 0);
}
}
/*
Expand Down Expand Up @@ -1016,6 +1025,10 @@ PHP_FUNCTION(twig_template_get_attributes)
return false;
}
if (isset($propertySandboxException)) {
throw $propertySandboxException;
}
if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
return null;
}
Expand All @@ -1036,6 +1049,11 @@ PHP_FUNCTION(twig_template_get_attributes)
efree(item);
RETURN_FALSE;
}
if (Z_TYPE_P(propertySandboxException) == IS_OBJECT) {
efree(item);
EG(exception) = propertySandboxException;
return;
}
if (ignoreStrictCheck || !TWIG_CALL_BOOLEAN(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "isStrictVariables" TSRMLS_CC)) {
efree(item);
return;
Expand All @@ -1053,11 +1071,15 @@ PHP_FUNCTION(twig_template_get_attributes)
}
/*
if ($this->env->hasExtension('sandbox')) {
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $method);
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $call ? '__call' : $method);
}
*/
MAKE_STD_ZVAL(zmethod);
ZVAL_STRING(zmethod, method, 1);
if (call) {
ZVAL_STRING(zmethod, "__call", 1);
} else {
ZVAL_STRING(zmethod, method, 1);
}
if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkMethodAllowed", object, zmethod TSRMLS_CC);
}
Expand All @@ -1074,26 +1096,32 @@ PHP_FUNCTION(twig_template_get_attributes)
try {
$ret = call_user_func_array(array($object, $method), $arguments);
} catch (BadMethodCallException $e) {
if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
return null;
}
throw $e;
if ($call && isset($propertySandboxException)) {
throw $propertySandboxException;
}
if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
return null;
}
throw $e;
}
*/
ret = TWIG_CALL_USER_FUNC_ARRAY(object, method, arguments TSRMLS_CC);
if (EG(exception) && TWIG_INSTANCE_OF(EG(exception), spl_ce_BadMethodCallException TSRMLS_CC)) {
efree(tmp_method_name_get);
efree(tmp_method_name_is);
efree(lcItem);
if (call && EG(exception) && TWIG_INSTANCE_OF(EG(exception), spl_ce_BadMethodCallException TSRMLS_CC)) {
if (Z_TYPE_P(propertySandboxException) == IS_OBJECT) {
efree(item);
EG(exception) = propertySandboxException;
return;
}
if (ignoreStrictCheck || !TWIG_CALL_BOOLEAN(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "isStrictVariables" TSRMLS_CC)) {
efree(tmp_method_name_get);
efree(tmp_method_name_is);
efree(lcItem);efree(item);
zend_clear_exception(TSRMLS_C);
return;
}
}
free_ret = 1;
efree(tmp_method_name_get);
efree(tmp_method_name_is);
efree(lcItem);
}
/*
// useful when calling a template method from a template
Expand Down
19 changes: 16 additions & 3 deletions lib/Twig/Template.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,15 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
}

if ($this->env->hasExtension('sandbox')) {
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
try {
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
} catch (Twig_Sandbox_SecurityError $propertySandboxException) {
}
}

return $object->$item;
if (!isset($propertySandboxException)) {
return $object->$item;
}
}
}

Expand Down Expand Up @@ -577,6 +582,10 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
return false;
}

if (isset($propertySandboxException)) {
throw $propertySandboxException;
}

if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
return;
}
Expand All @@ -589,14 +598,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;
}
Expand Down
35 changes: 35 additions & 0 deletions test/Twig/Tests/Extension/SandboxTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.bac }}',
'1_basic11' => '{{ obj.baz }}',
'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 %}",
Expand Down Expand Up @@ -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_basic10')->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');
Expand Down Expand Up @@ -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_basic11')->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()
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
}

0 comments on commit e20380f

Please sign in to comment.