From e6c000896c53d06af98f317fa6a7e6374908f6ae Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Jun 2022 11:33:20 +0900 Subject: [PATCH 1/5] feat: throw Exception if file path contains special char --- system/Autoloader/Autoloader.php | 14 ++++++++++---- tests/system/Autoloader/AutoloaderTest.php | 13 +++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/system/Autoloader/Autoloader.php b/system/Autoloader/Autoloader.php index 311428fe4165..6ab7804f8dd6 100644 --- a/system/Autoloader/Autoloader.php +++ b/system/Autoloader/Autoloader.php @@ -290,9 +290,9 @@ protected function includeFile(string $file) } /** - * Sanitizes a filename, replacing spaces with dashes. + * Check file path. * - * Removes special characters that are illegal in filenames on certain + * Checks special characters that are illegal in filenames on certain * operating systems and special characters requiring special escaping * to manipulate at the command line. Replaces spaces and consecutive * dashes with a single dash. Trim period, dash and underscore from beginning @@ -306,10 +306,16 @@ public function sanitizeFilename(string $filename): string // Plus the forward slash for directory separators since this might be a path. // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_278 // Modified to allow backslash and colons for on Windows machines. - $filename = preg_replace('/[^0-9\p{L}\s\/\-\_\.\:\\\\]/u', '', $filename); + $tmp = preg_replace('/[^0-9\p{L}\s\/\-\_\.\:\\\\]/u', '', $filename); // Clean up our filename edges. - return trim($filename, '.-_'); + $cleanFilename = trim($tmp, '.-_'); + + if ($filename !== $cleanFilename) { + throw new InvalidArgumentException('The file path contains special character that is not allowed: "' . $filename . '"'); + } + + return $cleanFilename; } private function loadComposerNamespaces(ClassLoader $composer): void diff --git a/tests/system/Autoloader/AutoloaderTest.php b/tests/system/Autoloader/AutoloaderTest.php index bcf91b90b679..0b33fa9ad0e3 100644 --- a/tests/system/Autoloader/AutoloaderTest.php +++ b/tests/system/Autoloader/AutoloaderTest.php @@ -16,6 +16,7 @@ use Config\Autoload; use Config\Modules; use Config\Services; +use InvalidArgumentException; use UnnamespacedClass; /** @@ -199,18 +200,18 @@ public function testloadClassNonNamespaced() public function testSanitizationSimply() { - $test = '${../path}!#/to/some/file.php_'; - $expected = '/path/to/some/file.php'; + $this->expectException(InvalidArgumentException::class); - $this->assertSame($expected, $this->loader->sanitizeFilename($test)); + $test = '${../path}!#/to/some/file.php_'; + + $this->loader->sanitizeFilename($test); } public function testSanitizationAllowUnicodeChars() { - $test = 'Ä/path/to/some/file.php_'; - $expected = 'Ä/path/to/some/file.php'; + $test = 'Ä/path/to/some/file.php'; - $this->assertSame($expected, $this->loader->sanitizeFilename($test)); + $this->assertSame($test, $this->loader->sanitizeFilename($test)); } public function testSanitizationAllowsWindowsFilepaths() From 62b20ac568ce17c6036ce7f35662e75ce87d9744 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 28 Jun 2022 15:01:53 +0900 Subject: [PATCH 2/5] feat: better exception message --- system/Autoloader/Autoloader.php | 19 ++++++++++++++++--- tests/system/Autoloader/AutoloaderTest.php | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/system/Autoloader/Autoloader.php b/system/Autoloader/Autoloader.php index 6ab7804f8dd6..677c8d3a8572 100644 --- a/system/Autoloader/Autoloader.php +++ b/system/Autoloader/Autoloader.php @@ -15,6 +15,7 @@ use Config\Autoload; use Config\Modules; use InvalidArgumentException; +use RuntimeException; /** * An autoloader that uses both PSR4 autoloading, and traditional classmaps. @@ -306,13 +307,25 @@ public function sanitizeFilename(string $filename): string // Plus the forward slash for directory separators since this might be a path. // http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_278 // Modified to allow backslash and colons for on Windows machines. - $tmp = preg_replace('/[^0-9\p{L}\s\/\-\_\.\:\\\\]/u', '', $filename); + $result = preg_match_all('/[^0-9\p{L}\s\/\-_.:\\\\]/u', $filename, $matches); + + if ($result > 0) { + $chars = implode('', $matches[0]); + + throw new InvalidArgumentException( + 'The file path contains special characters "' . $chars + . '" that are not allowed: "' . $filename . '"' + ); + } + if ($result === false) { + throw new RuntimeException(preg_last_error_msg() . ' filename: "' . $filename . '"'); + } // Clean up our filename edges. - $cleanFilename = trim($tmp, '.-_'); + $cleanFilename = trim($filename, '.-_'); if ($filename !== $cleanFilename) { - throw new InvalidArgumentException('The file path contains special character that is not allowed: "' . $filename . '"'); + throw new InvalidArgumentException('The characters ".-_" are not allowed in filename edges: "' . $filename . '"'); } return $cleanFilename; diff --git a/tests/system/Autoloader/AutoloaderTest.php b/tests/system/Autoloader/AutoloaderTest.php index 0b33fa9ad0e3..980f91756c5a 100644 --- a/tests/system/Autoloader/AutoloaderTest.php +++ b/tests/system/Autoloader/AutoloaderTest.php @@ -198,15 +198,30 @@ public function testloadClassNonNamespaced() $this->assertFalse($this->loader->loadClass('Modules')); } - public function testSanitizationSimply() + public function testSanitizationContailsSpecialChars() { $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage( + 'The file path contains special characters "${}!#" that are not allowed: "${../path}!#/to/some/file.php_"' + ); $test = '${../path}!#/to/some/file.php_'; $this->loader->sanitizeFilename($test); } + public function testSanitizationFilenameEdges() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage( + 'The characters ".-_" are not allowed in filename edges: "/path/to/some/file.php_"' + ); + + $test = '/path/to/some/file.php_'; + + $this->loader->sanitizeFilename($test); + } + public function testSanitizationAllowUnicodeChars() { $test = 'Ä/path/to/some/file.php'; From bbfd392db70b00a88beb8c9a1d13c56a8fc034b6 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Jul 2022 16:00:38 +0900 Subject: [PATCH 3/5] test: add test for regex error --- tests/system/Autoloader/AutoloaderTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/system/Autoloader/AutoloaderTest.php b/tests/system/Autoloader/AutoloaderTest.php index 980f91756c5a..215ad800a374 100644 --- a/tests/system/Autoloader/AutoloaderTest.php +++ b/tests/system/Autoloader/AutoloaderTest.php @@ -17,6 +17,7 @@ use Config\Modules; use Config\Services; use InvalidArgumentException; +use RuntimeException; use UnnamespacedClass; /** @@ -222,6 +223,16 @@ public function testSanitizationFilenameEdges() $this->loader->sanitizeFilename($test); } + public function testSanitizationRegexError() + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Malformed UTF-8 characters, possibly incorrectly encoded filename:'); + + $test = mb_convert_encoding('クラスファイル.php', 'EUC-JP', 'UTF-8'); + + $this->loader->sanitizeFilename($test); + } + public function testSanitizationAllowUnicodeChars() { $test = 'Ä/path/to/some/file.php'; From 307e19487249e582e2eda640e2fb56ffe843de9c Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Jul 2022 16:46:07 +0900 Subject: [PATCH 4/5] fix: preg_last_error_msg() can be used in PHP 8.0 or later --- system/Autoloader/Autoloader.php | 8 +++++++- tests/system/Autoloader/AutoloaderTest.php | 1 - 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/system/Autoloader/Autoloader.php b/system/Autoloader/Autoloader.php index 677c8d3a8572..4178068a3e9c 100644 --- a/system/Autoloader/Autoloader.php +++ b/system/Autoloader/Autoloader.php @@ -318,7 +318,13 @@ public function sanitizeFilename(string $filename): string ); } if ($result === false) { - throw new RuntimeException(preg_last_error_msg() . ' filename: "' . $filename . '"'); + if (version_compare(PHP_VERSION, '8.0.0', '>=')) { + $message = preg_last_error_msg(); + } else { + $message = 'Regex error. error code: ' . preg_last_error(); + } + + throw new RuntimeException($message . '. filename: "' . $filename . '"'); } // Clean up our filename edges. diff --git a/tests/system/Autoloader/AutoloaderTest.php b/tests/system/Autoloader/AutoloaderTest.php index 215ad800a374..2e0224709e49 100644 --- a/tests/system/Autoloader/AutoloaderTest.php +++ b/tests/system/Autoloader/AutoloaderTest.php @@ -226,7 +226,6 @@ public function testSanitizationFilenameEdges() public function testSanitizationRegexError() { $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Malformed UTF-8 characters, possibly incorrectly encoded filename:'); $test = mb_convert_encoding('クラスファイル.php', 'EUC-JP', 'UTF-8'); From d9c4d8cc3cb66e247fb87fa0f5ac3ceb81bb86a1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 1 Jul 2022 16:54:55 +0900 Subject: [PATCH 5/5] chore: skip UnwrapFutureCompatibleIfPhpVersionRector --- rector.php | 1 + 1 file changed, 1 insertion(+) diff --git a/rector.php b/rector.php index e83bfcc18c63..82e2a9e40057 100644 --- a/rector.php +++ b/rector.php @@ -92,6 +92,7 @@ // check on constant compare UnwrapFutureCompatibleIfPhpVersionRector::class => [ __DIR__ . '/system/CodeIgniter.php', + __DIR__ . '/system/Autoloader/Autoloader.php', ], // session handlers have the gc() method with underscored parameter `$max_lifetime`