Browse Source

Fix disallow -- string into filename for security purpose. Vulnerability
reported by Yılmaz Değirmenci

Laurent Destailleur 4 years ago
parent
commit
4fcd3fe493

+ 1 - 1
htdocs/admin/tools/export_files.php

@@ -34,7 +34,7 @@ $action = GETPOST('action', 'alpha');
 $what = GETPOST('what', 'alpha');
 $export_type = GETPOST('export_type', 'alpha');
 $file = trim(GETPOST('zipfilename_template', 'alpha'));
-$compression = GETPOST('compression');
+$compression = GETPOST('compression', 'aZ09');
 
 $file = dol_sanitizeFileName($file);
 $file = preg_replace('/(\.zip|\.tar|\.tgz|\.gz|\.tar\.gz|\.bz2)$/i', '', $file);

+ 3 - 2
htdocs/core/lib/functions.lib.php

@@ -866,8 +866,9 @@ function dol_sanitizeFileName($str, $newstr = '_', $unaccent = 1)
 	// List of special chars for filenames in windows are defined on page https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
 	// Char '>' '<' '|' '$' and ';' are special chars for shells.
 	// Char '/' and '\' are file delimiters.
-	$filesystem_forbidden_chars = array('<', '>', '/', '\\', '?', '*', '|', '"', ':', '°', '$', ';');
-	return dol_string_nospecial($unaccent ?dol_string_unaccent($str) : $str, $newstr, $filesystem_forbidden_chars);
+	// -- car can be used into filename to inject special paramaters like --use-compress-program to make command with file as parameter making remote execution of command
+	$filesystem_forbidden_chars = array('<', '>', '/', '\\', '?', '*', '|', '"', ':', '°', '$', ';', '--');
+	return dol_string_nospecial($unaccent ? dol_string_unaccent($str) : $str, $newstr, $filesystem_forbidden_chars);
 }
 
 /**

+ 24 - 1
test/phpunit/SecurityTest.php

@@ -244,7 +244,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase
 
         $login=checkLoginPassEntity('admin', 'admin', 1, array('dolibarr'));            // Should works because admin/admin exists
         print __METHOD__." login=".$login."\n";
-        $this->assertEquals($login, 'admin');
+        $this->assertEquals($login, 'admin', 'The test to check if pass of user "admin" is "admin" has failed');
 
         $login=checkLoginPassEntity('admin', 'admin', 1, array('http','dolibarr'));    // Should work because of second authetntication method
         print __METHOD__." login=".$login."\n";
@@ -326,4 +326,27 @@ class SecurityTest extends PHPUnit\Framework\TestCase
 		$result=restrictedArea($user, 'societe');
 		$this->assertEquals(1, $result);
     }
+
+    /**
+     * testDolSanitizeFileName
+     *
+     * @return void
+     */
+    public function testDolSanitizeFileName()
+    {
+    	global $conf,$user,$langs,$db;
+    	$conf=$this->savconf;
+    	$user=$this->savuser;
+    	$langs=$this->savlangs;
+    	$db=$this->savdb;
+
+    	//$dummyuser=new User($db);
+    	//$result=restrictedArea($dummyuser,'societe');
+
+    	$result=dol_sanitizeFileName('bad file | evilaction');
+    	$this->assertEquals('bad file _ evilaction', $result);
+
+    	$result=dol_sanitizeFileName('bad file --evilparam');
+    	$this->assertEquals('bad file _evilparam', $result);
+    }
 }