Merge pull request #79 from Icinga/fix/alphanum

Validate filenames before trying to store them
Icinga · Aug 12, 2024 · c4066bd · c4066bd
2 parents 93cc9b8 + 3f7d772
commit c4066bd
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 0 deletions.
diff --git a/library/Toplevelview/Model/View.php b/library/Toplevelview/Model/View.php
@@ -233,6 +233,22 @@ public function setText($text)
         return $this;
     }
 
+    /**
+     * validateName validates the name of the view.
+     * This can be used to ensure the YAML files have proper/expected names
+     * @return bool
+     */
+    public function validateName(): bool
+    {
+        if (empty($this->name)) {
+            return false;
+        }
+        if (preg_match('/[!@#\$%^&*\/\\\()]/', $this->name)) {
+            return false;
+        }
+        return true;
+    }
+
     /**
      * getName returns the name of this View
      * @return ?string

diff --git a/library/Toplevelview/ViewConfig.php b/library/Toplevelview/ViewConfig.php
@@ -242,10 +242,18 @@ public function loadAll($format = self::FORMAT_YAML): array
     /**
      * storeToSession stores a View's text to the user's session
      *
+     * @throws NotWritableError if the data cannot be stored
+     * @throws SecurityException if the user has no access to edit
+     *
      * @param $view
      */
     public function storeToSession($view): void
     {
+        // Assert the name is valid, to avoid tricky filenames such as '../../view'
+        if (!$view->validateName()) {
+            throw new NotWritableError('Invalid filename: %s', $view->getName());
+        }
+
         // Assert the user has rights to edit this view
         $restrictions = $this->getRestrictions('toplevelview/filter/edit');
         $this->assertAccessToView($restrictions, $view->getName());
@@ -266,10 +274,18 @@ public function clearSession($view): void
     /**
      * storeToFile stores a View to its configuration file
      *
+     * @throws NotWritableError if the data cannot be stored
+     * @throws SecurityException if the user has no access to edit
+     *
      * @param $view
      */
     public function storeToFile($view): void
     {
+        // Assert the name is valid, to avoid tricky filenames such as '../../view'
+        if (!$view->validateName()) {
+            throw new NotWritableError('Invalid filename: %s', $view->getName());
+        }
+
         // Assert the user has rights to edit this file
         $restrictions = $this->getRestrictions('toplevelview/filter/edit');
         $this->assertAccessToView($restrictions, $view->getName());

diff --git a/test/php/library/Toplevelview/Model/ViewTest.php b/test/php/library/Toplevelview/Model/ViewTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Tests\Icinga\Module\Toplevelview;
+
+use Icinga\Module\Toplevelview\Model\View;
+
+use PHPUnit\Framework\TestCase;
+
+final class ViewTest extends TestCase
+{
+    public function testViewValidateName()
+    {
+        $v = new View('myview', 'yml');
+        $this->assertSame('myview', $v->getName());
+
+        $tests = [
+            'example' => true,
+            'Example' => true,
+            'ex_ample' => true,
+            'ex_am-ple' => true,
+            '1ex_am-ple2' => true,
+            'Ex_a2m-ple123' => true,
+            '1ex_am-ple2' => true,
+            'example.yaml' => true,
+            'e#x(_)am_123-ple' => false,
+            'ex/ample' => false,
+            'ex\ample' => false,
+            '' => false,
+            'Филе' => true,
+            '😺' => true,
+            '../../example' => false
+        ];
+
+        foreach ($tests as $name => $value) {
+            $v->setName($name);
+            $this->assertSame($v->validateName(), $value);
+        }
+    }
+}