From cbf8dd439c5e56a56511e39180d014ce2ecd5221 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 6 Feb 2015 15:09:31 +0100 Subject: [PATCH] Normalize before processing --- lib/private/files/filesystem.php | 9 +++++---- lib/private/files/mapper.php | 6 +++++- tests/lib/files/filesystem.php | 22 ++++++++++++++++++++++ tests/lib/files/mapper.php | 16 +++++++++++++++- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php index 1ebc79086dd..492d9f1958c 100644 --- a/lib/private/files/filesystem.php +++ b/lib/private/files/filesystem.php @@ -524,9 +524,10 @@ class Filesystem { * @return bool */ static public function isFileBlacklisted($filename) { + $filename = self::normalizePath($filename); $blacklist = \OC_Config::getValue('blacklisted_files', array('.htaccess')); $filename = strtolower(basename($filename)); - return (in_array($filename, $blacklist)); + return in_array($filename, $blacklist); } /** @@ -700,6 +701,9 @@ class Filesystem { return '/'; } + //normalize unicode if possible + $path = \OC_Util::normalizeUnicode($path); + //no windows style slashes $path = str_replace('\\', '/', $path); @@ -736,9 +740,6 @@ class Filesystem { $path = substr($path, 0, -2); } - //normalize unicode if possible - $path = \OC_Util::normalizeUnicode($path); - return $windows_drive_letter . $path; } diff --git a/lib/private/files/mapper.php b/lib/private/files/mapper.php index 5e78ef03dd0..a950d78e33c 100644 --- a/lib/private/files/mapper.php +++ b/lib/private/files/mapper.php @@ -253,13 +253,17 @@ class Mapper // trim ending dots (for security reasons and win compatibility) $text = preg_replace('~\.+$~', '', $text); - if (empty($text)) { + if (empty($text) || \OC\Files\Filesystem::isFileBlacklisted($text)) { /** * Item slug would be empty. Previously we used uniqid() here. * However this means that the behaviour is not reproducible, so * when uploading files into a "empty" folder, the folders name is * different. * + * The other case is, that the slugified name would be a blacklisted + * filename. In this case we just use the same workaround by + * returning the secure md5 hash of the original name. + * * If there would be a md5() hash collision, the deduplicate check * will spot this and append an index later, so this should not be * a problem. diff --git a/tests/lib/files/filesystem.php b/tests/lib/files/filesystem.php index 88e98fbb8c6..0a5ae40d994 100644 --- a/tests/lib/files/filesystem.php +++ b/tests/lib/files/filesystem.php @@ -108,6 +108,28 @@ class Filesystem extends \Test\TestCase { } } + public function isFileBlacklistedData() { + return array( + array('/etc/foo/bar/foo.txt', false), + array('\etc\foo/bar\foo.txt', false), + array('.htaccess', true), + array('.htaccess/', true), + array('.htaccess\\', true), + array('/etc/foo\bar/.htaccess\\', true), + array('/etc/foo\bar/.htaccess/', true), + array('/etc/foo\bar/.htaccess/foo', false), + array('//foo//bar/\.htaccess/', true), + array('\foo\bar\.HTAccess', true), + ); + } + + /** + * @dataProvider isFileBlacklistedData + */ + public function testIsFileBlacklisted($path, $expected) { + $this->assertSame($expected, \OC\Files\Filesystem::isFileBlacklisted($path)); + } + public function testNormalizeWindowsPaths() { $this->assertEquals('/', \OC\Files\Filesystem::normalizePath('')); $this->assertEquals('/', \OC\Files\Filesystem::normalizePath('\\')); diff --git a/tests/lib/files/mapper.php b/tests/lib/files/mapper.php index 48ae95b7e72..d786de235cb 100644 --- a/tests/lib/files/mapper.php +++ b/tests/lib/files/mapper.php @@ -59,6 +59,20 @@ class Mapper extends \PHPUnit_Framework_TestCase { $this->assertEquals('D:/folder.name.with.peri-ods/te-st-2.t-x-t', $this->mapper->slugifyPath('D:/folder.name.with.peri ods/te st.t x t', 2)); $this->assertEquals('D:/folder.name.with.peri-ods/te-st.t-x-t', $this->mapper->slugifyPath('D:/folder.name.with.peri ods/te st.t x t')); - + // files with special characters + $this->assertEquals('D:/' . md5('ありがとう'), $this->mapper->slugifyPath('D:/ありがとう')); + $this->assertEquals('D:/' . md5('ありがとう') . '/issue6722.txt', $this->mapper->slugifyPath('D:/ありがとう/issue6722.txt')); + + // blacklisted files + $this->assertEquals('D:/' . md5('.htaccess'), $this->mapper->slugifyPath('D:/.htaccess')); + $this->assertEquals('D:/' . md5('.htaccess.'), $this->mapper->slugifyPath('D:/.htaccess.')); + $this->assertEquals('D:/' . md5('.htAccess'), $this->mapper->slugifyPath('D:/.htAccess')); + $this->assertEquals('D:/' . md5('.htAccess\\…\\') . '/a', $this->mapper->slugifyPath('D:/.htAccess\…\/とa')); + $this->assertEquals('D:/' . md5('.htaccess-'), $this->mapper->slugifyPath('D:/.htaccess-')); + $this->assertEquals('D:/' . md5('.htaあccess'), $this->mapper->slugifyPath('D:/.htaあccess')); + $this->assertEquals('D:/' . md5(' .htaccess'), $this->mapper->slugifyPath('D:/ .htaccess')); + $this->assertEquals('D:/' . md5('.htaccess '), $this->mapper->slugifyPath('D:/.htaccess ')); + $this->assertEquals('D:/' . md5(' .htaccess '), $this->mapper->slugifyPath('D:/ .htaccess ')); + } } -- 2.39.5