Added extra checks for invalid file chars in newfile.php and newfolder.php

- added PHP utility function to check for file name validity
- fixes issue where a user can create a file called ".." from the files UI
- added extra checks to make sure newfile.php and newfolder.php also
  check for invalid characters

diff --git a/apps/files/ajax/newfile.php b/apps/files/ajax/newfile.php
index 1853098c5072cd06b9d158f41c178356b7d6a851..0187b20075920e4920177c2c08b22b2263ee178a 100644 (file)
--- a/apps/files/ajax/newfile.php
+++ b/apps/files/ajax/newfile.php
@@ -50,16 +50,22 @@ $l10n = \OC_L10n::get('files');
 $result = array(
        'success'       => false,
        'data'          => NULL
-       );
+);
+$trimmedFileName = trim($filename);
 
-if(trim($filename) === '') {
+if($trimmedFileName === '') {
        $result['data'] = array('message' => (string)$l10n->t('File name cannot be empty.'));
        OCP\JSON::error($result);
        exit();
 }
+if($trimmedFileName === '.' || $trimmedFileName === '..') {
+       $result['data'] = array('message' => (string)$l10n->t('"%s" is an invalid file name.', $trimmedFileName));
+       OCP\JSON::error($result);
+       exit();
+}
 
-if(strpos($filename, '/') !== false) {
-       $result['data'] = array('message' => (string)$l10n->t('File name must not contain "/". Please choose a different name.'));
+if(!OCP\Util::isValidFileName($filename)) {
+       $result['data'] = array('message' => (string)$l10n->t("Invalid name, '\\', '/', '<', '>', ':', '\"', '|', '?' and '*' are not allowed."));
        OCP\JSON::error($result);
        exit();
 }

diff --git a/apps/files/ajax/newfolder.php b/apps/files/ajax/newfolder.php
index 4cfcae3090d58da8ad3201480d95278c30851a97..b2b4fb27f74a0a3bdd4ae4a2a8cdd3df543214bc 100644 (file)
--- a/apps/files/ajax/newfolder.php
+++ b/apps/files/ajax/newfolder.php
@@ -23,8 +23,8 @@ if(trim($foldername) === '') {
        exit();
 }
 
-if(strpos($foldername, '/') !== false) {
-       $result['data'] = array('message' => $l10n->t('Folder name must not contain "/". Please choose a different name.'));
+if(!OCP\Util::isValidFileName($foldername)) {
+       $result['data'] = array('message' => (string)$l10n->t("Invalid name, '\\', '/', '<', '>', ':', '\"', '|', '?' and '*' are not allowed."));
        OCP\JSON::error($result);
        exit();
 }

diff --git a/lib/private/util.php b/lib/private/util.php
index 0585749d615f62e67cc03a30384582cc2f78b387..bdc762d4bef89ee6d254265090f3a5d41ea7f6b2 100755 (executable)
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -1149,4 +1149,25 @@ class OC_Util {
                }
                return $version;
        }
+
+       /**
+        * Returns whether the given file name is valid
+        * @param $file string file name to check
+        * @return bool true if the file name is valid, false otherwise
+        */
+       public static function isValidFileName($file) {
+               $trimmed = trim($file);
+               if ($trimmed === '') {
+                       return false;
+               }
+               if ($trimmed === '.' || $trimmed === '..') {
+                       return false;
+               }
+               foreach (str_split($trimmed) as $char) {
+                       if (strpos(\OCP\FILENAME_INVALID_CHARS, $char) !== false) {
+                               return false;
+                       }
+               }
+               return true;
+       }
 }

diff --git a/lib/public/constants.php b/lib/public/constants.php
index 1495c620dc9a65fbb3cea4c7e82f2531d190bf81..350646a0ac0a5f280c6ec16a7d68912a77ce8f80 100644 (file)
--- a/lib/public/constants.php
+++ b/lib/public/constants.php
@@ -35,3 +35,6 @@ const PERMISSION_UPDATE = 2;
 const PERMISSION_DELETE = 8;
 const PERMISSION_SHARE = 16;
 const PERMISSION_ALL = 31;
+
+const FILENAME_INVALID_CHARS = "\\/<>:\"|?*\n";
+

diff --git a/lib/public/util.php b/lib/public/util.php
index 570283e2a8a424efe230f2acd4dd8cf7564c3834..585c5d226341785dea867c33fb91e0e0310f521f 100644 (file)
--- a/lib/public/util.php
+++ b/lib/public/util.php
@@ -486,4 +486,13 @@ class Util {
        public static function uploadLimit() {
                return \OC_Helper::uploadLimit();
        }
+
+       /**
+        * Returns whether the given file name is valid
+        * @param $file string file name to check
+        * @return bool true if the file name is valid, false otherwise
+        */
+       public static function isValidFileName($file) {
+               return \OC_Util::isValidFileName($file);
+       }
 }

diff --git a/tests/lib/util.php b/tests/lib/util.php
index bfe68f5f680fa348dfbfb2b6145129b43be1972e..ee336aa111891410e90ddea2b06b9b22ef8ee3ed 100644 (file)
--- a/tests/lib/util.php
+++ b/tests/lib/util.php
@@ -170,4 +170,52 @@ class Test_Util extends PHPUnit_Framework_TestCase {
                        array('442aa682de2a64db1e010f50e60fd9c9', 'local::C:\Users\ADMINI~1\AppData\Local\Temp\2/442aa682de2a64db1e010f50e60fd9c9/')
                );
        }
+
+       /**
+        * @dataProvider filenameValidationProvider
+        */
+       public function testFilenameValidation($file, $valid) {
+               // private API
+               $this->assertEquals($valid, \OC_Util::isValidFileName($file));
+               // public API
+               $this->assertEquals($valid, \OCP\Util::isValidFileName($file));
+       }
+
+       public function filenameValidationProvider() {
+               return array(
+                       // valid names
+                       array('boringname', true),
+                       array('something.with.extension', true),
+                       array('now with spaces', true),
+                       array('.a', true),
+                       array('..a', true),
+                       array('.dotfile', true),
+                       array('single\'quote', true),
+                       array('  spaces before', true),
+                       array('spaces after   ', true),
+                       array('allowed chars including the crazy ones $%&_-^@!,()[]{}=;#', true),
+                       array('汉字也能用', true),
+                       array('und Ümläüte sind auch willkommen', true),
+                       // disallowed names
+                       array('', false),
+                       array('     ', false),
+                       array('.', false),
+                       array('..', false),
+                       array('back\\slash', false),
+                       array('sl/ash', false),
+                       array('lt<lt', false),
+                       array('gt>gt', false),
+                       array('col:on', false),
+                       array('double"quote', false),
+                       array('pi|pe', false),
+                       array('dont?ask?questions?', false),
+                       array('super*star', false),
+                       array('new\nline', false),
+                       // better disallow these to avoid unexpected trimming to have side effects
+                       array(' ..', false),
+                       array('.. ', false),
+                       array('. ', false),
+                       array(' .', false),
+               );
+       }
 }