Skip to content

Commit ebb14a3

Browse files
hasonhason
committed
Fixed sandbox for methods
1 parent eb6e05e commit ebb14a3

File tree

3 files changed

+99
-23
lines changed

3 files changed

+99
-23
lines changed

‎ext/twig/twig.c‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,7 @@ PHP_FUNCTION(twig_template_get_attributes)
710710
char *class_name = NULL;
711711
zval *tmp_class;
712712
char *type_name;
713+
zval *propertySandboxException;
713714

714715
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ozz|asbb", &template, &object, &zitem, &arguments, &type, &type_len, &isDefinedTest, &ignoreStrictCheck) == FAILURE) {
715716
return;
@@ -917,10 +918,15 @@ PHP_FUNCTION(twig_template_get_attributes)
917918
}
918919
919920
if ($this->env->hasExtension('sandbox')) {
920-
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
921+
try {
922+
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
923+
} catch (Twig_Sandbox_SecurityError $propertySandboxException) {
924+
}
921925
}
922926
923-
return $object->$item;
927+
if (!isset($propertySandboxException)) {
928+
return $object->$item;
929+
}
924930
}
925931
}
926932
*/
@@ -938,14 +944,17 @@ PHP_FUNCTION(twig_template_get_attributes)
938944
if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
939945
TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkPropertyAllowed", object, zitem TSRMLS_CC);
940946
}
941-
if (EG(exception)) {
947+
if (EG(exception) && TWIG_INSTANCE_OF_USERLAND(EG(exception), "Twig_Sandbox_SecurityError" TSRMLS_CC)) {
948+
propertySandboxException = EG(exception);
949+
EG(exception) = NULL;
950+
} else if (EG(exception)) {
942951
efree(item);
943952
return;
953+
} else {
954+
ret = TWIG_PROPERTY(object, zitem TSRMLS_CC);
955+
efree(item);
956+
RETURN_ZVAL(ret, 1, 0);
944957
}
945-
946-
ret = TWIG_PROPERTY(object, zitem TSRMLS_CC);
947-
efree(item);
948-
RETURN_ZVAL(ret, 1, 0);
949958
}
950959
}
951960
/*
@@ -1016,6 +1025,10 @@ PHP_FUNCTION(twig_template_get_attributes)
10161025
return false;
10171026
}
10181027
1028+
if (isset($propertySandboxException)) {
1029+
throw $propertySandboxException;
1030+
}
1031+
10191032
if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
10201033
return null;
10211034
}
@@ -1036,6 +1049,11 @@ PHP_FUNCTION(twig_template_get_attributes)
10361049
efree(item);
10371050
RETURN_FALSE;
10381051
}
1052+
if (Z_TYPE_P(propertySandboxException) == IS_OBJECT) {
1053+
efree(item);
1054+
EG(exception) = propertySandboxException;
1055+
return;
1056+
}
10391057
if (ignoreStrictCheck || !TWIG_CALL_BOOLEAN(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "isStrictVariables" TSRMLS_CC)) {
10401058
efree(item);
10411059
return;
@@ -1053,11 +1071,15 @@ PHP_FUNCTION(twig_template_get_attributes)
10531071
}
10541072
/*
10551073
if ($this->env->hasExtension('sandbox')) {
1056-
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $method);
1074+
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $call ? '__call' : $method);
10571075
}
10581076
*/
10591077
MAKE_STD_ZVAL(zmethod);
1060-
ZVAL_STRING(zmethod, method, 1);
1078+
if (call) {
1079+
ZVAL_STRING(zmethod, "__call", 1);
1080+
} else {
1081+
ZVAL_STRING(zmethod, method, 1);
1082+
}
10611083
if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
10621084
TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkMethodAllowed", object, zmethod TSRMLS_CC);
10631085
}
@@ -1074,26 +1096,32 @@ PHP_FUNCTION(twig_template_get_attributes)
10741096
try {
10751097
$ret = call_user_func_array(array($object, $method), $arguments);
10761098
} catch (BadMethodCallException $e) {
1077-
if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
1078-
return null;
1079-
}
1080-
throw $e;
1099+
if ($call && isset($propertySandboxException)) {
1100+
throw $propertySandboxException;
1101+
}
1102+
1103+
if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
1104+
return null;
1105+
}
1106+
throw $e;
10811107
}
10821108
*/
10831109
ret = TWIG_CALL_USER_FUNC_ARRAY(object, method, arguments TSRMLS_CC);
1084-
if (EG(exception) && TWIG_INSTANCE_OF(EG(exception), spl_ce_BadMethodCallException TSRMLS_CC)) {
1110+
efree(tmp_method_name_get);
1111+
efree(tmp_method_name_is);
1112+
efree(lcItem);
1113+
if (call && EG(exception) && TWIG_INSTANCE_OF(EG(exception), spl_ce_BadMethodCallException TSRMLS_CC)) {
1114+
if (Z_TYPE_P(propertySandboxException) == IS_OBJECT) {
1115+
efree(item);
1116+
EG(exception) = propertySandboxException;
1117+
return;
1118+
}
10851119
if (ignoreStrictCheck || !TWIG_CALL_BOOLEAN(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "isStrictVariables" TSRMLS_CC)) {
1086-
efree(tmp_method_name_get);
1087-
efree(tmp_method_name_is);
1088-
efree(lcItem);efree(item);
10891120
zend_clear_exception(TSRMLS_C);
10901121
return;
10911122
}
10921123
}
10931124
free_ret = 1;
1094-
efree(tmp_method_name_get);
1095-
efree(tmp_method_name_is);
1096-
efree(lcItem);
10971125
}
10981126
/*
10991127
// useful when calling a template method from a template

‎lib/Twig/Template.php‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,10 +530,15 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
530530
}
531531

532532
if ($this->env->hasExtension('sandbox')) {
533-
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
533+
try {
534+
$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
535+
} catch (Twig_Sandbox_SecurityError $propertySandboxException) {
536+
}
534537
}
535538

536-
return $object->$item;
539+
if (!isset($propertySandboxException)) {
540+
return $object->$item;
541+
}
537542
}
538543
}
539544

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

585+
if (isset($propertySandboxException)) {
586+
throw $propertySandboxException;
587+
}
588+
580589
if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
581590
return;
582591
}
@@ -589,14 +598,18 @@ protected function getAttribute($object, $item, array $arguments = array(), $typ
589598
}
590599

591600
if ($this->env->hasExtension('sandbox')) {
592-
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $method);
601+
$this->env->getExtension('sandbox')->checkMethodAllowed($object, $call ? '__call' : $method);
593602
}
594603

595604
// Some objects throw exceptions when they have __call, and the method we try
596605
// to call is not supported. If ignoreStrictCheck is true, we should return null.
597606
try {
598607
$ret = call_user_func_array(array($object, $method), $arguments);
599608
} catch (BadMethodCallException $e) {
609+
if ($call && isset($propertySandboxException)) {
610+
throw $propertySandboxException;
611+
}
612+
600613
if ($call && ($ignoreStrictCheck || !$this->env->isStrictVariables())) {
601614
return;
602615
}

‎test/Twig/Tests/Extension/SandboxTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ protected function setUp()
3131
'1_basic7' => '{{ cycle(["foo","bar"], 1) }}',
3232
'1_basic8' => '{{ obj.getfoobar }}{{ obj.getFooBar }}',
3333
'1_basic9' => '{{ obj.foobar }}{{ obj.fooBar }}',
34+
'1_basic10' => '{{ obj.bac }}',
35+
'1_basic11' => '{{ obj.baz }}',
36+
'1_basic12' => '{{ obj.xyz }}',
3437
'1_basic' => '{% if obj.foo %}{{ obj.foo|upper }}{% endif %}',
3538
'1_layout' => '{% block content %}{% endblock %}',
3639
'1_child' => "{% extends \"1_layout\" %}\n{% block content %}\n{{ \"a\"|json_encode }}\n{% endblock %}",
@@ -101,6 +104,14 @@ public function testSandboxGloballySet()
101104
} catch (Twig_Sandbox_SecurityError $e) {
102105
}
103106

107+
$twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => '__call'));
108+
try {
109+
$twig->loadTemplate('1_basic10')->render(self::$params);
110+
$this->fail('Sandbox throws a SecurityError exception if an unallowed property is called in the template through a method (__call)');
111+
} catch (Twig_Sandbox_SecurityError $e) {
112+
$this->assertEquals('Calling "bac" property on a "FooObject" object is not allowed.', $e->getRawMessage());
113+
}
114+
104115
$twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => 'foo'));
105116
FooObject::reset();
106117
$this->assertEquals('foo', $twig->loadTemplate('1_basic1')->render(self::$params), 'Sandbox allow some methods');
@@ -136,6 +147,12 @@ public function testSandboxGloballySet()
136147

137148
$this->assertEquals('foobarfoobar', $twig->loadTemplate('1_basic9')->render(self::$params), 'Sandbox allow methods via shortcut names (ie. without get/set)');
138149
}
150+
151+
$twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => 'getBaz'));
152+
$this->assertEquals('baz', $twig->loadTemplate('1_basic11')->render(self::$params), 'Sandbox allow some methods');
153+
154+
$twig = $this->getEnvironment(true, array(), self::$templates, array(), array(), array('FooObject' => '__call'));
155+
$this->assertEquals('xyz', $twig->loadTemplate('1_basic12')->render(self::$params), 'Sandbox allow a method (__call())');
139156
}
140157

141158
public function testSandboxLocallySetForAnInclude()
@@ -192,6 +209,10 @@ class FooObject
192209

193210
public $bar = 'bar';
194211

212+
public $baz = 'baz';
213+
214+
public $bac = 'bac';
215+
195216
public static function reset()
196217
{
197218
self::$called = array('__toString' => 0, 'foo' => 0, 'getFooBar' => 0);
@@ -217,4 +238,18 @@ public function getFooBar()
217238

218239
return 'foobar';
219240
}
241+
242+
public function getBaz()
243+
{
244+
return $this->baz;
245+
}
246+
247+
public function __call($name, $arguments)
248+
{
249+
if ('bac' === strtolower($name)) {
250+
throw new BadMethodCallException();
251+
}
252+
253+
return $name;
254+
}
220255
}

0 commit comments

Comments
 (0)