Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

MDL-29749 files: Improved params PARAM_PATH and PARAM_FILE

The logic change here allows for multiple dots to be part
of a file name or folder name. Most Unit Tests have not
been altered and reflect the exact logic as it was before.
  • Loading branch information...
commit caf16a57e56373a938eddce2030249a2e1648559 1 parent ca48fe5
@FMCorz FMCorz authored
View
4 lib/filelib.php
@@ -546,11 +546,11 @@ function file_get_user_used_space() {
* @param string $str
* @return string path
*/
-function file_correct_filepath($str) { //TODO: what is this? (skodak)
+function file_correct_filepath($str) { //TODO: what is this? (skodak) - No idea (Fred)
if ($str == '/' or empty($str)) {
return '/';
} else {
- return '/'.trim($str, './@#$ ').'/';
+ return '/'.trim($str, '/').'/';
}
}
View
22 lib/moodlelib.php
@@ -920,8 +920,7 @@ function clean_param($param, $type) {
case PARAM_FILE: // Strip all suspicious characters from filename
$param = fix_utf8($param);
$param = preg_replace('~[[:cntrl:]]|[&<>"`\|\':\\\\/]~u', '', $param);
- $param = preg_replace('~\.\.+~', '', $param);
- if ($param === '.') {
+ if ($param === '.' || $param === '..') {
$param = '';
}
return $param;
@@ -929,10 +928,23 @@ function clean_param($param, $type) {
case PARAM_PATH: // Strip all suspicious characters from file path
$param = fix_utf8($param);
$param = str_replace('\\', '/', $param);
- $param = preg_replace('~[[:cntrl:]]|[&<>"`\|\':]~u', '', $param);
- $param = preg_replace('~\.\.+~', '', $param);
+
+ // Explode the path and clean each element using the PARAM_FILE rules.
+ $breadcrumb = explode('/', $param);
+ foreach ($breadcrumb as $key => $crumb) {
+ if ($crumb === '.' && $key === 0) {
+ // Special condition to allow for relative current path such as ./currentdirfile.txt.
+ } else {
+ $crumb = clean_param($crumb, PARAM_FILE);
+ }
+ $breadcrumb[$key] = $crumb;
+ }
+ $param = implode('/', $breadcrumb);
+
+ // Remove multiple current path (./././) and multiple slashes (///).
$param = preg_replace('~//+~', '/', $param);
- return preg_replace('~/(\./)+~', '/', $param);
+ $param = preg_replace('~/(\./)+~', '/', $param);
+ return $param;
case PARAM_HOST: // allow FQDN or IPv4 dotted quad
$param = preg_replace('/[^\.\d\w-]/','', $param ); // only allowed chars
View
41 lib/tests/moodlelib_test.php
@@ -872,7 +872,26 @@ function test_clean_param_localurl() {
function test_clean_param_file() {
$this->assertEquals(clean_param('correctfile.txt', PARAM_FILE), 'correctfile.txt');
$this->assertEquals(clean_param('b\'a<d`\\/fi:l>e.t"x|t', PARAM_FILE), 'badfile.txt');
- $this->assertEquals(clean_param('../parentdirfile.txt', PARAM_FILE), 'parentdirfile.txt');
+ $this->assertEquals(clean_param('../parentdirfile.txt', PARAM_FILE), '..parentdirfile.txt');
+ $this->assertEquals(clean_param('../../grandparentdirfile.txt', PARAM_FILE), '....grandparentdirfile.txt');
+ $this->assertEquals(clean_param('..\winparentdirfile.txt', PARAM_FILE), '..winparentdirfile.txt');
+ $this->assertEquals(clean_param('../../wingrandparentdir.txt', PARAM_FILE), '....wingrandparentdir.txt');
+ $this->assertEquals(clean_param('myfile.a.b.txt', PARAM_FILE), 'myfile.a.b.txt');
+ $this->assertEquals(clean_param('myfile..a..b.txt', PARAM_FILE), 'myfile..a..b.txt');
+ $this->assertEquals(clean_param('myfile.a..b...txt', PARAM_FILE), 'myfile.a..b...txt');
+ $this->assertEquals(clean_param('myfile.a.txt', PARAM_FILE), 'myfile.a.txt');
+ $this->assertEquals(clean_param('myfile...txt', PARAM_FILE), 'myfile...txt');
+ $this->assertEquals(clean_param('...jpg', PARAM_FILE), '...jpg');
+ $this->assertEquals(clean_param('.a.b.', PARAM_FILE), '.a.b.');
+ $this->assertEquals(clean_param('.', PARAM_FILE), '');
+ $this->assertEquals(clean_param('..', PARAM_FILE), '');
+ $this->assertEquals(clean_param('...', PARAM_FILE), '...');
+ $this->assertEquals(clean_param('. . . .', PARAM_FILE), '. . . .');
+ $this->assertEquals(clean_param('dontrtrim.me. .. .. . ', PARAM_FILE), 'dontrtrim.me. .. .. . ');
+ $this->assertEquals(clean_param(' . .dontltrim.me', PARAM_FILE), ' . .dontltrim.me');
+ $this->assertEquals(clean_param("here is a tab\t.txt", PARAM_FILE), 'here is a tab.txt');
+ $this->assertEquals(clean_param("here is a line\r\nbreak.txt", PARAM_FILE), 'here is a linebreak.txt');
+
//The following behaviours have been maintained although they seem a little odd
$this->assertEquals(clean_param('funny:thing', PARAM_FILE), 'funnything');
$this->assertEquals(clean_param('./currentdirfile.txt', PARAM_FILE), '.currentdirfile.txt');
@@ -881,6 +900,26 @@ function test_clean_param_file() {
$this->assertEquals(clean_param('~/myfile.txt', PARAM_FILE), '~myfile.txt');
}
+ function test_clean_param_path() {
+ $this->assertEquals(clean_param('correctfile.txt', PARAM_PATH), 'correctfile.txt');
+ $this->assertEquals(clean_param('b\'a<d`\\/fi:l>e.t"x|t', PARAM_PATH), 'bad/file.txt');
+ $this->assertEquals(clean_param('../parentdirfile.txt', PARAM_PATH), '/parentdirfile.txt');
+ $this->assertEquals(clean_param('../../grandparentdirfile.txt', PARAM_PATH), '/grandparentdirfile.txt');
+ $this->assertEquals(clean_param('..\winparentdirfile.txt', PARAM_PATH), '/winparentdirfile.txt');
+ $this->assertEquals(clean_param('../../wingrandparentdir.txt', PARAM_PATH), '/wingrandparentdir.txt');
+ $this->assertEquals(clean_param('funny:thing', PARAM_PATH), 'funnything');
+ $this->assertEquals(clean_param('./././here', PARAM_PATH), './here');
+ $this->assertEquals(clean_param('./currentdirfile.txt', PARAM_PATH), './currentdirfile.txt');
+ $this->assertEquals(clean_param('c:\temp\windowsfile.txt', PARAM_PATH), 'c/temp/windowsfile.txt');
+ $this->assertEquals(clean_param('/home/user/linuxfile.txt', PARAM_PATH), '/home/user/linuxfile.txt');
+ $this->assertEquals(clean_param('/home../user ./.linuxfile.txt', PARAM_PATH), '/home../user ./.linuxfile.txt');
+ $this->assertEquals(clean_param('~/myfile.txt', PARAM_PATH), '~/myfile.txt');
+ $this->assertEquals(clean_param('~/../myfile.txt', PARAM_PATH), '~/myfile.txt');
+ $this->assertEquals(clean_param('/..b../.../myfile.txt', PARAM_PATH), '/..b../.../myfile.txt');
+ $this->assertEquals(clean_param('..b../.../myfile.txt', PARAM_PATH), '..b../.../myfile.txt');
+ $this->assertEquals(clean_param('/super//slashes///', PARAM_PATH), '/super/slashes/');
+ }
+
function test_clean_param_username() {
global $CFG;
$currentstatus = $CFG->extendedusernamechars;
Please sign in to comment.
Something went wrong with that request. Please try again.