From 797e0a614cc44e627a54dfd39ce4047d176ebd9b Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 10 Jan 2014 16:14:37 +0100 Subject: [PATCH] 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 --- apps/files/ajax/newfile.php | 14 +++++++--- apps/files/ajax/newfolder.php | 4 +-- lib/private/util.php | 21 +++++++++++++++ lib/public/constants.php | 3 +++ lib/public/util.php | 9 +++++++ tests/lib/util.php | 48 +++++++++++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 6 deletions(-) diff --git a/apps/files/ajax/newfile.php b/apps/files/ajax/newfile.php index 1853098c507..0187b200759 100644 --- 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 4cfcae3090d..b2b4fb27f74 100644 --- 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 0585749d615..bdc762d4bef 100755 --- 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 1495c620dc9..350646a0ac0 100644 --- 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 570283e2a8a..585c5d22634 100644 --- 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 bfe68f5f680..ee336aa1118 100644 --- 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('ltgt', 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), + ); + } } -- 2.39.5