Merge branch '68137-master-prevent-removing-all-dots-in-filename' of https://github.com/DSI-Universite-Rennes2/moodle

lib/filestorage/file_archive.php

@@ -152,7 +152,8 @@ abstract class file_archive implements Iterator {
             }
         }
 
-        $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 === '.') {

lib/filestorage/zip_archive.php

@@ -145,7 +145,8 @@ class zip_archive extends file_archive {
      */
     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 === '.') {

lib/tests/filestorage_zip_archive_test.php

@@ -0,0 +1,86 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * 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'],
+            ['../', ''],
+            ['.../...//', ''],
+            ['.', ''],
+        ];
+    }
+}