Merge branch '2.x' into 3.x

* 2.x: Fix premature loop exit in Security Policy lookup of allowed methods/properties
twigphp · Oct 27, 2023 · 248855b · 248855b
2 parents 415f4cc + 02262de
commit 248855b
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 6 deletions.
diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php
@@ -94,9 +94,8 @@ public function checkMethodAllowed($obj, $method): void
         $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): void
     {
         $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
@@ -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];