[BUGFIX] Allow unicode characters in verifyFileNameAgainstDenyPattern 72/58772/2
authorPascal Rinker <info@crynton.com>
Mon, 25 Jun 2018 14:43:32 +0000 (16:43 +0200)
committerBenni Mack <benni@typo3.org>
Sun, 28 Oct 2018 16:44:29 +0000 (17:44 +0100)
Using (valid) unicode characters in
GeneralUtility::verifyFilenameAgainstDenyPattern was not possible due
to a missing unicode modifier when evaluating regular expressions.
The unicode modifier has been added.
Since unicode errors in regular expressions will lead to `false`
results, it is important to perform type-safe checks against `0`.

Resolves: #67061
Releases: master, 8.7
Change-Id: If3eea7129c92b296b85b93a1f1c81a446a2f5f90
Reviewed-on: https://review.typo3.org/58772
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/core/Classes/Utility/GeneralUtility.php
typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php

index c6a5518..79f7ece 100644 (file)
@@ -3404,9 +3404,9 @@ class GeneralUtility
     {
         $pattern = '/[[:cntrl:]]/';
         if ((string)$filename !== '' && (string)$GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'] !== '') {
-            $pattern = '/(?:[[:cntrl:]]|' . $GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'] . ')/i';
+            $pattern = '/(?:[[:cntrl:]]|' . $GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'] . ')/iu';
         }
-        return !preg_match($pattern, $filename);
+        return preg_match($pattern, $filename) === 0;
     }
 
     /**
index 8de3aca..703719d 100644 (file)
@@ -4522,34 +4522,120 @@ class GeneralUtilityTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
     /**
      * @return array
      */
-    public function deniedFilesDataProvider()
+    public function deniedFilesWithoutDenyPatternDataProvider(): array
     {
         return [
             'Nul character in file' => ['image' . chr(0) . '.gif'],
             'Nul character in file with .php' => ['image.php' . chr(0) . '.gif'],
-            'Regular .php file' => ['file.php'],
-            'Regular .php5 file' => ['file.php5'],
-            'Regular .php3 file' => ['file.php3'],
-            'Regular .phpsh file' => ['file.phpsh'],
-            'Regular .phtml file' => ['file.phtml'],
-            'Regular .pht file' => ['file.pht'],
-            'PHP file in the middle' => ['file.php.txt'],
-            '.htaccess file' => ['.htaccess'],
+            'Nul character in file with \\0' => ['image' . "\0" . '.gif'],
+            'Nul character in file with \\0 with .php' => ['image.php' . "\0" . '.gif'],
+            'Nul character and UTF-8 in file' => ['Ссылка' . "\0" . '.gif'],
+            'Nul character and Latin-1 in file' => ['ÉÐØ' . "\0" . '.gif'],
         ];
     }
 
     /**
+     * Tests whether verifyFilenameAgainstDenyPattern detects files with nul character without file deny pattern.
+     *
+     * @param string $deniedFile
+     * @test
+     * @dataProvider deniedFilesWithoutDenyPatternDataProvider
+     */
+    public function verifyNulCharacterFilesAgainstPatternWithoutFileDenyPattern(string $deniedFile)
+    {
+        $GLOBALS['TYPO3_CONF_VARS']['BE']['fileDenyPattern'] = '';
+        $this->assertFalse(GeneralUtility::verifyFilenameAgainstDenyPattern($deniedFile));
+    }
+
+    /**
+     * @return array
+     */
+    public function deniedFilesWithDefaultDenyPatternDataProvider(): array
+    {
+        $data = [
+            'Nul character in file' => ['image' . "\0", '.gif'],
+            'Nul character in file with .php' => ['image.php' . "\0", '.gif'],
+            'Nul character and UTF-8 in file' => ['Ссылка' . "\0", '.gif'],
+            'Nul character and Latin-1 in file' => ['ÉÐØ' . "\0", '.gif'],
+            'Regular .php file' => ['file' , '.php'],
+            'Lower umlaut .php file' => ['üWithFile', '.php'],
+            'Upper umlaut .php file' => ['fileWithÜ', '.php'],
+            'invalid UTF-8-sequence' => ["\xc0" . 'file', '.php'],
+            'Could be overlong NUL in some UTF-8 implementations, invalid in RFC3629' => ["\xc0\x80" . 'file', '.php'],
+            'Regular .php5 file' => ['file', '.php5'],
+            'Regular .php3 file' => ['file', '.php3'],
+            'Regular .phpsh file' => ['file', '.phpsh'],
+            'Regular .phtml file' => ['file', '.phtml'],
+            'Regular .pht file' => ['file', '.pht'],
+            'PHP file in the middle' => ['file', '.php.txt'],
+            '.htaccess file' => ['', '.htaccess'],
+        ];
+
+        // Mixing with regular utf-8
+        $utf8Characters = 'Ссылка';
+        foreach ($data as $key => $value) {
+            if ($value[0] === '') {
+                continue;
+            }
+            $data[$key . ' with UTF-8 characters prepended'] = [$utf8Characters . $value[0], $value[1]];
+            $data[$key . ' with UTF-8 characters appended'] = [$value[0] . $utf8Characters, $value[1]];
+        }
+
+        // combine to single value
+        $data = array_map(
+            function (array $values): array {
+                return [implode('', $values)];
+            },
+            $data
+        );
+
+        // Encoding with UTF-16
+        foreach ($data as $key => $value) {
+            $data[$key . ' encoded with UTF-16'] = [mb_convert_encoding($value[0], 'UTF-16')];
+        }
+
+        return $data;
+    }
+
+    /**
      * Tests whether verifyFilenameAgainstDenyPattern detects denied files.
      *
      * @param string $deniedFile
      * @test
-     * @dataProvider deniedFilesDataProvider
+     * @dataProvider deniedFilesWithDefaultDenyPatternDataProvider
      */
     public function verifyFilenameAgainstDenyPatternDetectsNotAllowedFiles($deniedFile)
     {
         $this->assertFalse(GeneralUtility::verifyFilenameAgainstDenyPattern($deniedFile));
     }
 
+    /**
+     * @return array
+     */
+    public function allowedFilesDataProvider(): array
+    {
+        return [
+            'Regular .gif file' => ['image.gif'],
+            'Regular uppercase .gif file' => ['IMAGE.gif'],
+            'UTF-8 .gif file' => ['Ссылка.gif'],
+            'Lower umlaut .jpg file' => ['üWithFile.jpg'],
+            'Upper umlaut .png file' => ['fileWithÜ.png'],
+            'Latin-1 .gif file' => ['ÉÐØ.gif']
+        ];
+    }
+
+    /**
+     * Tests whether verifyFilenameAgainstDenyPattern accepts allowed files.
+     *
+     * @param string $allowedFile
+     * @test
+     * @dataProvider allowedFilesDataProvider
+     */
+    public function verifyFilenameAgainstDenyPatternAcceptAllowedFiles(string $allowedFile)
+    {
+        $this->assertTrue(GeneralUtility::verifyFilenameAgainstDenyPattern($allowedFile));
+    }
+
     /////////////////////////////////////////////////////////////////////////////////////
     // Tests concerning copyDirectory
     /////////////////////////////////////////////////////////////////////////////////////