Skip to content

Commit

Permalink
MDL-68137 core_files: replace consecutive dots in filename by single dot
Browse files Browse the repository at this point in the history
  • Loading branch information
jboulen committed Apr 28, 2020
1 parent 9df4a4d commit 7d918bc
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 2 deletions.
3 changes: 2 additions & 1 deletion lib/filestorage/file_archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '.') {
Expand Down
3 changes: 2 additions & 1 deletion lib/filestorage/zip_archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '.') {
Expand Down
86 changes: 86 additions & 0 deletions lib/tests/filestorage_zip_archive_test.php
Original file line number Diff line number Diff line change
@@ -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'],
['../', ''],
['.../...//', ''],
['.', ''],
];
}
}

0 comments on commit 7d918bc

Please sign in to comment.