Check exact extension in checkFilename utility (#3061)

* Fix uploads_dangerous_extensions checking (#3060) * Remove redundant prefixing of `.` to extension (#3060)
getgrav · Nov 17, 2020 · ae6f0b5 · ae6f0b5
1 parent 247d1a9
commit ae6f0b5
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 7 deletions.
diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php
@@ -928,11 +928,7 @@ public static function getMimeByLocalFile($filename, $default = 'application/oct
     public static function checkFilename($filename)
     {
         $dangerous_extensions = Grav::instance()['config']->get('security.uploads_dangerous_extensions', []);
-        array_walk($dangerous_extensions, function (&$val) {
-            $val = '.' . $val;
-        });
-
-        $extension = '.' . pathinfo($filename, PATHINFO_EXTENSION);
+        $extension = pathinfo($filename, PATHINFO_EXTENSION);
 
         return !(
             // Empty filenames are not allowed.
@@ -941,8 +937,8 @@ public static function checkFilename($filename)
             || strtr($filename, "\t\v\n\r\0\\/", '_______') !== $filename
             // Filename should not start or end with dot or space.
             || trim($filename, '. ') !== $filename
-            // Filename should not contain .php in it.
-            || static::contains($extension, $dangerous_extensions)
+            // File extension should not be part of configured dangerous extensions
+            || in_array($extension, $dangerous_extensions)
         );
     }
 

diff --git a/tests/unit/Grav/Common/UtilsTest.php b/tests/unit/Grav/Common/UtilsTest.php
@@ -540,4 +540,20 @@ public function testUrlwithExternals()
         $this->assertSame('//foo.com', Utils::url('//foo.com'));
         $this->assertSame('//foo.com?param=x', Utils::url('//foo.com?param=x'));
     }
+
+    public function testCheckFilename()
+    {
+        // configure extension for consistent results
+        /** @var \Grav\Common\Config\Config $config */
+        $config = $this->grav['config'];
+        $config->set('security.uploads_dangerous_extensions', ['php', 'html', 'htm', 'exe', 'js']);
+
+        $this->assertFalse(Utils::checkFilename('foo.php'));
+        $this->assertFalse(Utils::checkFilename('bar.js'));
+
+        $this->assertTrue(Utils::checkFilename('foo.json'));
+        $this->assertTrue(Utils::checkFilename('foo.xml'));
+        $this->assertTrue(Utils::checkFilename('foo.yaml'));
+        $this->assertTrue(Utils::checkFilename('foo.yml'));
+    }
 }