From 4652d203e37d06b427872888ccb17227c1e0818b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 2 Nov 2016 09:23:01 +0100 Subject: [PATCH 1/2] Make sure we don't scan files that can not be accessed Signed-off-by: Joas Schilling --- lib/private/Files/Cache/Scanner.php | 21 +++++++++++++++++++++ tests/lib/Files/Cache/ScannerTest.php | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 28f7be0b65..237934db7a 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -132,6 +132,24 @@ class Scanner extends BasicEmitter implements IScanner { */ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) { + if (!\OC::$server->getDatabaseConnection()->supports4ByteText()) { + // verify database - e.g. mysql only 3-byte chars + if (preg_match('%(?: + \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 + | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 + | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 +)%xs', $file)) { + // 4-byte characters are not supported in file names + return null; + } + } + + try { + $this->storage->verifyPath(dirname($file), basename($file)); + } catch (\Exception $e) { + return null; + } + // only proceed if $file is not a partial file nor a blacklisted file if (!self::isPartialFile($file) and !Filesystem::isFileBlacklisted($file)) { @@ -167,6 +185,9 @@ class Scanner extends BasicEmitter implements IScanner { // scan the parent if it's not in the cache (id -1) and the current file is not the root folder if ($file and $parentId === -1) { $parentData = $this->scanFile($parent); + if (!$parentData) { + return null; + } $parentId = $parentData['fileid']; } if ($parent) { diff --git a/tests/lib/Files/Cache/ScannerTest.php b/tests/lib/Files/Cache/ScannerTest.php index b44b6f5d0f..075716f803 100644 --- a/tests/lib/Files/Cache/ScannerTest.php +++ b/tests/lib/Files/Cache/ScannerTest.php @@ -70,6 +70,32 @@ class ScannerTest extends \Test\TestCase { $this->assertEquals($cachedData['mimetype'], 'image/png'); } + function testFile4Byte() { + $data = "dummy file data\n"; + $this->storage->file_put_contents('foo🙈.txt', $data); + + if (\OC::$server->getDatabaseConnection()->supports4ByteText()) { + $this->assertNotNull($this->scanner->scanFile('foo🙈.txt')); + $this->assertTrue($this->cache->inCache('foo🙈.txt'), true); + + $cachedData = $this->cache->get('foo🙈.txt'); + $this->assertEquals(strlen($data), $cachedData['size']); + $this->assertEquals('text/plain', $cachedData['mimetype']); + $this->assertNotEquals(-1, $cachedData['parent']); //parent folders should be scanned automatically + } else { + $this->assertNull($this->scanner->scanFile('foo🙈.txt')); + $this->assertFalse($this->cache->inCache('foo🙈.txt'), true); + } + } + + function testFileInvalidChars() { + $data = "dummy file data\n"; + $this->storage->file_put_contents("foo\nbar.txt", $data); + + $this->assertNull($this->scanner->scanFile("foo\nbar.txt")); + $this->assertFalse($this->cache->inCache("foo\nbar.txt"), true); + } + private function fillTestFolders() { $textData = "dummy file data\n"; $imgData = file_get_contents(\OC::$SERVERROOT . '/core/img/logo.png'); From 558f169671208fb349bb40de7b6e0abb02097832 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 9 Nov 2016 10:58:11 +0100 Subject: [PATCH 2/2] Move the validation into one place only Signed-off-by: Joas Schilling --- lib/private/Files/Cache/Scanner.php | 19 ++------- lib/private/Files/Storage/Common.php | 25 ++++++++++++ lib/private/Files/View.php | 40 +++++++------------ lib/public/Files/EmptyFileNameException.php | 31 ++++++++++++++ .../Files/InvalidDirectoryException.php | 31 ++++++++++++++ tests/lib/Files/PathVerificationTest.php | 2 +- 6 files changed, 106 insertions(+), 42 deletions(-) create mode 100644 lib/public/Files/EmptyFileNameException.php create mode 100644 lib/public/Files/InvalidDirectoryException.php diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 237934db7a..8625e4904c 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -131,25 +131,14 @@ class Scanner extends BasicEmitter implements IScanner { * @throws \OCP\Lock\LockedException */ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) { - - if (!\OC::$server->getDatabaseConnection()->supports4ByteText()) { - // verify database - e.g. mysql only 3-byte chars - if (preg_match('%(?: - \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 - | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 - | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 -)%xs', $file)) { - // 4-byte characters are not supported in file names + if ($file !== '') { + try { + $this->storage->verifyPath(dirname($file), basename($file)); + } catch (\Exception $e) { return null; } } - try { - $this->storage->verifyPath(dirname($file), basename($file)); - } catch (\Exception $e) { - return null; - } - // only proceed if $file is not a partial file nor a blacklisted file if (!self::isPartialFile($file) and !Filesystem::isFileBlacklisted($file)) { diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index c975791295..5561f6a889 100644 --- a/lib/private/Files/Storage/Common.php +++ b/lib/private/Files/Storage/Common.php @@ -45,8 +45,10 @@ use OC\Files\Cache\Scanner; use OC\Files\Cache\Updater; use OC\Files\Filesystem; use OC\Files\Cache\Watcher; +use OCP\Files\EmptyFileNameException; use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; +use OCP\Files\InvalidDirectoryException; use OCP\Files\InvalidPathException; use OCP\Files\ReservedWordException; use OCP\Files\Storage\ILockingStorage; @@ -487,8 +489,31 @@ abstract class Common implements Storage, ILockingStorage { /** * @inheritdoc + * @throws InvalidPathException */ public function verifyPath($path, $fileName) { + + // verify empty and dot files + $trimmed = trim($fileName); + if ($trimmed === '') { + throw new EmptyFileNameException(); + } + + if (\OC\Files\Filesystem::isIgnoredDir($trimmed)) { + throw new InvalidDirectoryException(); + } + + if (!\OC::$server->getDatabaseConnection()->supports4ByteText()) { + // verify database - e.g. mysql only 3-byte chars + if (preg_match('%(?: + \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 + | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 + | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 +)%xs', $fileName)) { + throw new InvalidCharacterInPathException(); + } + } + if (isset($fileName[255])) { throw new FileNameTooLongException(); } diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 7866f90157..67f8918099 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -53,8 +53,10 @@ use OC\Files\Storage\Storage; use OC\User\User; use OCP\Constants; use OCP\Files\Cache\ICacheEntry; +use OCP\Files\EmptyFileNameException; use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; +use OCP\Files\InvalidDirectoryException; use OCP\Files\InvalidPathException; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; @@ -1788,39 +1790,25 @@ class View { * @throws InvalidPathException */ public function verifyPath($path, $fileName) { - - $l10n = \OC::$server->getL10N('lib'); - - // verify empty and dot files - $trimmed = trim($fileName); - if ($trimmed === '') { - throw new InvalidPathException($l10n->t('Empty filename is not allowed')); - } - if (\OC\Files\Filesystem::isIgnoredDir($trimmed)) { - throw new InvalidPathException($l10n->t('Dot files are not allowed')); - } - - if (!\OC::$server->getDatabaseConnection()->supports4ByteText()) { - // verify database - e.g. mysql only 3-byte chars - if (preg_match('%(?: - \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 - | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 - | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 -)%xs', $fileName)) { - throw new InvalidPathException($l10n->t('4-byte characters are not supported in file names')); - } - } - try { /** @type \OCP\Files\Storage $storage */ list($storage, $internalPath) = $this->resolvePath($path); $storage->verifyPath($internalPath, $fileName); } catch (ReservedWordException $ex) { - throw new InvalidPathException($l10n->t('File name is a reserved word')); + $l = \OC::$server->getL10N('lib'); + throw new InvalidPathException($l->t('File name is a reserved word')); } catch (InvalidCharacterInPathException $ex) { - throw new InvalidPathException($l10n->t('File name contains at least one invalid character')); + $l = \OC::$server->getL10N('lib'); + throw new InvalidPathException($l->t('File name contains at least one invalid character')); } catch (FileNameTooLongException $ex) { - throw new InvalidPathException($l10n->t('File name is too long')); + $l = \OC::$server->getL10N('lib'); + throw new InvalidPathException($l->t('File name is too long')); + } catch (InvalidDirectoryException $ex) { + $l = \OC::$server->getL10N('lib'); + throw new InvalidPathException($l->t('Dot files are not allowed')); + } catch (EmptyFileNameException $ex) { + $l = \OC::$server->getL10N('lib'); + throw new InvalidPathException($l->t('Empty filename is not allowed')); } } diff --git a/lib/public/Files/EmptyFileNameException.php b/lib/public/Files/EmptyFileNameException.php new file mode 100644 index 0000000000..22cdd5e805 --- /dev/null +++ b/lib/public/Files/EmptyFileNameException.php @@ -0,0 +1,31 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP\Files; + +/** + * Class EmptyFileNameException + * + * @package OCP\Files + * @since 9.2.0 + */ +class EmptyFileNameException extends InvalidPathException { +} diff --git a/lib/public/Files/InvalidDirectoryException.php b/lib/public/Files/InvalidDirectoryException.php new file mode 100644 index 0000000000..a82dc0829e --- /dev/null +++ b/lib/public/Files/InvalidDirectoryException.php @@ -0,0 +1,31 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCP\Files; + +/** + * Class InvalidDirectoryException + * + * @package OCP\Files + * @since 9.2.0 + */ +class InvalidDirectoryException extends InvalidPathException { +} diff --git a/tests/lib/Files/PathVerificationTest.php b/tests/lib/Files/PathVerificationTest.php index 12285bb7ac..c1cebe975f 100644 --- a/tests/lib/Files/PathVerificationTest.php +++ b/tests/lib/Files/PathVerificationTest.php @@ -86,7 +86,7 @@ class PathVerificationTest extends \Test\TestCase { if (!$connection->supports4ByteText()) { $this->expectException(InvalidPathException::class); - $this->expectExceptionMessage('4-byte characters are not supported in file names'); + $this->expectExceptionMessage('File name contains at least one invalid character'); } $this->view->verifyPath('', $fileName);