diff --git a/lib/filestorage/file_archive.php b/lib/filestorage/file_archive.php index 8d9ab818c89f3..77768feeb67fd 100644 --- a/lib/filestorage/file_archive.php +++ b/lib/filestorage/file_archive.php @@ -152,7 +152,8 @@ protected function mangle_pathname($localname) { } } - $result = preg_replace('/\.\.+/', '', $result); + $result = preg_replace('/\.\.+\//', '', $result); // Cleanup any potential ../ transversal (any number of dots). + $result = preg_replace('/\.\.+/', '.', $result); // Join together any number of consecutive dots. $result = ltrim($result); // no leading / if ($result === '.') { diff --git a/lib/filestorage/zip_archive.php b/lib/filestorage/zip_archive.php index 213131436421b..61b7d336d65b0 100644 --- a/lib/filestorage/zip_archive.php +++ b/lib/filestorage/zip_archive.php @@ -145,7 +145,8 @@ public function open($archivepathname, $mode=file_archive::CREATE, $encoding=nul */ protected function mangle_pathname($localname) { $result = str_replace('\\', '/', $localname); // no MS \ separators - $result = preg_replace('/\.\.+/', '', $result); // prevent /.../ + $result = preg_replace('/\.\.+\//', '', $result); // Cleanup any potential ../ transversal (any number of dots). + $result = preg_replace('/\.\.+/', '.', $result); // Join together any number of consecutive dots. $result = ltrim($result, '/'); // no leading slash if ($result === '.') { diff --git a/lib/tests/filestorage_zip_archive_test.php b/lib/tests/filestorage_zip_archive_test.php new file mode 100644 index 0000000000000..339919f5501c0 --- /dev/null +++ b/lib/tests/filestorage_zip_archive_test.php @@ -0,0 +1,86 @@ +. + +/** + * Unit tests for /lib/filestorage/zip_archive.php. + * + * @package core_files + * @copyright 2020 Université Rennes 2 {@link https://www.univ-rennes2.fr} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; + +require_once($CFG->libdir . '/filestorage/zip_archive.php'); + +/** + * Unit tests for /lib/filestorage/zip_archive.php. + * + * @package core_files + * @copyright 2020 Université Rennes 2 {@link https://www.univ-rennes2.fr} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class filestorage_zip_archive_testcase extends advanced_testcase { + /** + * Test mangle_pathname() method. + * + * @dataProvider pathname_provider + * + * @param string $string Parameter sent to mangle_pathname method. + * @param string $expected Expected return value. + */ + public function test_mangle_pathname($string, $expected) { + $ziparchive = new zip_archive(); + + $method = new ReflectionMethod('zip_archive', 'mangle_pathname'); + $method->setAccessible(true); + + $result = $method->invoke($ziparchive, $string); + $this->assertSame($expected, $result); + } + + /** + * Provide some tested pathnames and expected results. + * + * @return array Array of tested pathnames and expected results. + */ + public function pathname_provider() { + return [ + // Test a string. + ['my file.pdf', 'my file.pdf'], + + // Test a string with MS separator. + ['c:\temp\my file.pdf', 'c:/temp/my file.pdf'], + + // Test a string with 2 consecutive dots. + ['my file..pdf', 'my file.pdf'], + + // Test a string with 3 consecutive dots. + ['my file...pdf', 'my file.pdf'], + + // Test a string beginning with leading slash. + ['/tmp/my file.pdf', 'tmp/my file.pdf'], + + // Test some path traversal attacks. + ['../../../../../etc/passwd', 'etc/passwd'], + ['../', ''], + ['.../...//', ''], + ['.', ''], + ]; + } +}