diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php index 1406e8061a9..3b79a870f83 100644 --- a/src/Sandbox/SecurityPolicy.php +++ b/src/Sandbox/SecurityPolicy.php @@ -94,9 +94,8 @@ public function checkMethodAllowed($obj, $method) $allowed = false; $method = strtr($method, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz'); foreach ($this->allowedMethods as $class => $methods) { - if ($obj instanceof $class) { - $allowed = \in_array($method, $methods); - + if ($obj instanceof $class && \in_array($method, $methods)) { + $allowed = true; break; } } @@ -111,9 +110,8 @@ public function checkPropertyAllowed($obj, $property) { $allowed = false; foreach ($this->allowedProperties as $class => $properties) { - if ($obj instanceof $class) { - $allowed = \in_array($property, \is_array($properties) ? $properties : [$properties]); - + if ($obj instanceof $class && \in_array($property, \is_array($properties) ? $properties : [$properties])) { + $allowed = true; break; } } diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index e365da63280..dff2ddc8dd4 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -36,6 +36,7 @@ protected function setUp(): void 'name' => 'Fabien', 'obj' => new FooObject(), 'arr' => ['obj' => new FooObject()], + 'child_obj' => new ChildClass(), ]; self::$templates = [ @@ -56,6 +57,8 @@ protected function setUp(): void '1_range_operator' => '{{ (1..2)[0] }}', '1_syntax_error_wrapper' => '{% sandbox %}{% include "1_syntax_error" %}{% endsandbox %}', '1_syntax_error' => '{% syntax error }}', + '1_childobj_parentmethod' => '{{ child_obj.ParentMethod() }}', + '1_childobj_childmethod' => '{{ child_obj.ChildMethod() }}', ]; } @@ -410,6 +413,42 @@ public function testSandboxWithClosureFilter() $this->assertSame('foo, bar', $twig->load('index')->render([])); } + public function testMultipleClassMatchesViaInheritanceInAllowedMethods() + { + $twig_child_first = $this->getEnvironment(true, [], self::$templates, [], [], [ + 'Twig\Tests\Extension\ChildClass' => ['ChildMethod'], + 'Twig\Tests\Extension\ParentClass' => ['ParentMethod'], + ]); + $twig_parent_first = $this->getEnvironment(true, [], self::$templates, [], [], [ + 'Twig\Tests\Extension\ParentClass' => ['ParentMethod'], + 'Twig\Tests\Extension\ChildClass' => ['ChildMethod'], + ]); + + try { + $twig_child_first->load('1_childobj_childmethod')->render(self::$params); + } catch (SecurityError $e) { + $this->fail('This test case is malfunctioning as even the child class method which comes first is not being allowed.'); + } + + try { + $twig_parent_first->load('1_childobj_parentmethod')->render(self::$params); + } catch (SecurityError $e) { + $this->fail('This test case is malfunctioning as even the parent class method which comes first is not being allowed.'); + } + + try { + $twig_parent_first->load('1_childobj_childmethod')->render(self::$params); + } catch (SecurityError $e) { + $this->fail('checkMethodAllowed is exiting prematurely after matching a parent class and not seeing a method allowed on a child class later in the list'); + } + + try { + $twig_child_first->load('1_childobj_parentmethod')->render(self::$params); + } catch (SecurityError $e) { + $this->fail('checkMethodAllowed is exiting prematurely after matching a child class and not seeing a method allowed on its parent class later in the list'); + } + } + protected function getEnvironment($sandboxed, $options, $templates, $tags = [], $filters = [], $methods = [], $properties = [], $functions = []) { $loader = new ArrayLoader($templates); @@ -421,6 +460,19 @@ protected function getEnvironment($sandboxed, $options, $templates, $tags = [], } } +class ParentClass +{ + public function ParentMethod() + { + } +} +class ChildClass extends ParentClass +{ + public function ChildMethod() + { + } +} + class FooObject { public static $called = ['__toString' => 0, 'foo' => 0, 'getFooBar' => 0];