Add allowList functionality to Twig security policy (#1293)

wintercms · Feb 5, 2025 · c231c40 · c231c40
1 parent 37d0c36
commit c231c40
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 23 deletions.
diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php
@@ -309,15 +309,7 @@ public function runPage($page, $useAjax = true)
         /*
          * The 'this' variable is reserved for default variables.
          */
-        $this->getTwig()->addGlobal('this', [
-            'page'        => $this->page,
-            'layout'      => $this->layout,
-            'theme'       => $this->theme,
-            'param'       => $this->router->getParameters(),
-            'controller'  => $this,
-            'environment' => App::environment(),
-            'session'     => App::make('session'),
-        ]);
+        $this->getTwig()->addGlobal('this', $this->getControllerGlobalVars());
 
         /*
          * Add global vars defined by View::share() into Twig, only if they have yet to be specified.
@@ -1249,6 +1241,22 @@ public function getTwig()
         return $this->twig;
     }
 
+    /**
+     * Returns the globals to be provided to twig
+     */
+    public function getControllerGlobalVars()
+    {
+        return [
+            'page'        => $this->page ?? new Page(),
+            'layout'      => $this->layout ?? new Layout(),
+            'theme'       => $this->theme,
+            'param'       => $this->router->getParameters(),
+            'controller'  => $this,
+            'environment' => App::environment(),
+            'session'     => App::make('session'),
+        ];
+    }
+
     /**
      * Returns the Twig loader.
      * @return \Cms\Twig\Loader

diff --git a/modules/system/tests/twig/SecurityPolicyTest.php b/modules/system/tests/twig/SecurityPolicyTest.php
@@ -24,6 +24,45 @@ public function testCannotGetTwigInstanceFromCmsController()
         ');
     }
 
+    public function testAllowedMethods()
+    {
+        // put, get
+        $value = trim($this->renderTwigInCmsController('
+            {{ this.session.put("test", "value") }}
+            {{ this.session.get("test", "default") }}
+        '));
+        $this->assertEquals("value", $value);
+
+        // has
+        $value = trim($this->renderTwigInCmsController('
+            {{ this.session.put("test", "value") }}
+            {% if this.session.has("test") %}success{% else %}failure{% endif %}
+        '));
+        $this->assertEquals("success", $value);
+
+        // forget
+        $value = trim($this->renderTwigInCmsController('
+            {{ this.session.put("test", "value") }}
+            {{ this.session.forget("test") }}
+            {% if this.session.has("test") %}failure{% else %}success{% endif %}
+        '));
+        $this->assertEquals("success", $value);
+
+        // flush
+        $value = trim($this->renderTwigInCmsController('
+            {{ this.session.put("test", "value") }}
+            {{ this.session.flush() }}
+            {% if this.session.has("test") %}failure{% else %}success{% endif %}
+        '));
+        $this->assertEquals("success", $value);
+
+        // Test all other methods blocked
+        $this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
+        $this->renderTwigInCmsController('
+            {{ this.session.driver }}
+        ');
+    }
+
     public function testCannotGetTwigLoaderFromCmsController()
     {
         $this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
@@ -86,6 +125,17 @@ public function testCanReadFromAModel()
         $this->assertEquals('value', $result);
     }
 
+    public function testCannotAccessModelQuery()
+    {
+        $this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
+
+        $this->renderTwigInCmsController('
+            {{ dump(model.getQuery) }}
+        ', [
+            'model' => new \Winter\Storm\Database\Model(),
+        ]);
+    }
+
     public function testCannotFillAModel()
     {
         $this->expectException(\Twig\Sandbox\SecurityNotAllowedMethodError::class);
@@ -258,12 +308,11 @@ protected function renderTwigInCmsController(string $source, array $vars = [])
         $controller = new Controller();
         $twig = $controller->getTwig();
         $template = $twig->createTemplate($source, 'test.case');
+
         return $twig->render($template, [
-            'this' => [
-                'controller' => $controller,
-                'page' => new Page(),
-                'theme' => new Theme()
-            ],
+            'this' => array_merge($controller->getControllerGlobalVars(), [
+                'theme' => new Theme(),
+            ]),
         ] + $vars);
     }
 }
diff --git a/modules/system/twig/SecurityPolicy.php b/modules/system/twig/SecurityPolicy.php
@@ -5,15 +5,16 @@
 use Cms\Classes\Controller;
 use Cms\Classes\Theme;
 use Illuminate\Database\Eloquent\Model as DbModel;
-use Winter\Storm\Halcyon\Model as HalcyonModel;
+use Illuminate\Session\SessionManager;
 use Twig\Markup;
 use Twig\Sandbox\SecurityNotAllowedFunctionError;
-use Twig\Template;
-use Twig\Sandbox\SecurityPolicyInterface;
 use Twig\Sandbox\SecurityNotAllowedMethodError;
 use Twig\Sandbox\SecurityNotAllowedPropertyError;
 use Twig\Sandbox\SecurityNotAllowedTagError;
+use Twig\Sandbox\SecurityPolicyInterface;
+use Twig\Template;
 use Winter\Storm\Halcyon\Datasource\DatasourceInterface;
+use Winter\Storm\Halcyon\Model as HalcyonModel;
 
 /**
  * SecurityPolicy globally blocks accessibility of certain methods and properties.
@@ -60,6 +61,7 @@ final class SecurityPolicy implements SecurityPolicyInterface
             'update',
             'delete',
             'forceDelete',
+            'getQuery',
         ],
         HalcyonModel::class => [
             'fill',
@@ -72,6 +74,7 @@ final class SecurityPolicy implements SecurityPolicyInterface
             'update',
             'delete',
             'forceDelete',
+            'getQuery',
         ],
         DatasourceInterface::class => [
             'insert',
@@ -90,6 +93,19 @@ final class SecurityPolicy implements SecurityPolicyInterface
         ],
     ];
 
+    /**
+     * @var array<string, string[]> List of allowed methods, grouped by applicable instance.
+     */
+    protected $allowedMethods = [
+        SessionManager::class => [
+            'put',
+            'get',
+            'has',
+            'forget',
+            'flush',
+        ],
+    ];
+
     /**
      * @var array<string, string[]> List of forbidden properties, grouped by applicable instance.
      */
@@ -104,12 +120,16 @@ final class SecurityPolicy implements SecurityPolicyInterface
      */
     public function __construct()
     {
-        foreach ($this->blockedMethods as $type => $methods) {
-            $this->blockedMethods[$type] = array_map('strtolower', $methods);
-        }
-
-        foreach ($this->blockedProperties as $type => $properties) {
-            $this->blockedProperties[$type] = array_map('strtolower', $properties);
+        $properties = [
+            'blockedMethods',
+            'allowedMethods',
+            'blockedProperties',
+        ];
+
+        foreach ($properties as $property) {
+            foreach ($this->{$property} as $type => $values) {
+                $this->{$property}[$type] = array_map('strtolower', $values);
+            }
         }
     }
 
@@ -174,6 +194,13 @@ public function checkMethodAllowed($obj, $method): void
             throw new SecurityNotAllowedMethodError(sprintf('Calling "%s" method on a "%s" object is blocked.', $method, $class), $class, $method);
         }
 
+        foreach ($this->allowedMethods as $type => $methods) {
+            if ($obj instanceof $type && !in_array($method, $methods)) {
+                $class = get_class($obj);
+                throw new SecurityNotAllowedMethodError(sprintf('Calling "%s" method on a "%s" object is blocked.', $method, $class), $class, $method);
+            }
+        }
+
         foreach ($this->blockedMethods as $type => $methods) {
             if ($obj instanceof $type && in_array($method, $methods)) {
                 $class = get_class($obj);