Skip to content

Exclude Directories #1627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mmattel wants to merge 2 commits into from
Closed

Exclude Directories #1627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a241926
Feature PR, references: Exclude Directories (merged, for professional…
mmattel Oct 5, 2016
76ad849
Corrections according nickvergessen suggestions
mmattel Oct 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/dav/lib/Connector/Sabre/CustomPropertiesBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ public function propFind($path, PropFind $propFind) {
} catch (ServiceUnavailable $e) {
// might happen for unavailable mount points, skip
return;
} catch (Forbidden $e) {
// might happen for excluded mount points, skip
return;
} catch (NotFound $e) {
// in some rare (buggy) cases the node might not be found,
// we catch the exception to prevent breaking the whole list with a 404
Expand Down
11 changes: 11 additions & 0 deletions apps/dav/lib/Connector/Sabre/Directory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ public function __construct($view, $info, $tree = null, $shareManager = null) {
*/
public function createFile($name, $data = null) {

// the check here is necessary, because createFile uses put covered in sabre/file.php
// and not touch covered in files/view.php
if (\OC\Files\Filesystem::isForbiddenFileOrDir($name)) {
throw new \Sabre\DAV\Exception\Forbidden();
}
try {
// for chunked upload also updating a existing file is a "createFile"
// because we create all the chunks before re-assemble them to the existing file.
Expand Down Expand Up @@ -156,7 +161,13 @@ public function createFile($name, $data = null) {
* @throws \Sabre\DAV\Exception\ServiceUnavailable
*/
public function createDirectory($name) {

try {
# the check here is necessary, because createDirectory does not use the methods in files/view.php
if (\OC\Files\Filesystem::isForbiddenFileOrDir($name)) {
throw new \Sabre\DAV\Exception\Forbidden();
}

if (!$this->info->isCreatable()) {
throw new \Sabre\DAV\Exception\Forbidden();
}
Expand Down
23 changes: 22 additions & 1 deletion apps/dav/lib/Connector/Sabre/ObjectTree.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,16 @@ public function cacheNode(Node $node) {
* @throws InvalidPath
* @throws \Sabre\DAV\Exception\Locked
* @throws \Sabre\DAV\Exception\NotFound
* @throws \Sabre\DAV\Exception\Forbidden
* @throws \Sabre\DAV\Exception\ServiceUnavailable
*/
public function getNodeForPath($path) {
// check the path, also called when the path has been entered manually eg via a file explorer
if (\OC\Files\Filesystem::isForbiddenFileOrDir($path)) {
throw new \Sabre\DAV\Exception\Forbidden();
}


if (!$this->fileView) {
throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup');
}
Expand Down Expand Up @@ -203,6 +210,11 @@ public function move($sourcePath, $destinationPath) {
throw new \Sabre\DAV\Exception\ServiceUnavailable('filesystem not setup');
}

# check the destination path, for source see below
if (\OC\Files\Filesystem::isForbiddenFileOrDir($destinationPath)) {
throw new \Sabre\DAV\Exception\Forbidden();
}

$infoDestination = $this->fileView->getFileInfo(dirname($destinationPath));
if (dirname($destinationPath) === dirname($sourcePath)) {
$sourcePermission = $infoDestination && $infoDestination->isUpdateable();
Expand All @@ -222,6 +234,9 @@ public function move($sourcePath, $destinationPath) {
}

$targetNodeExists = $this->nodeExists($destinationPath);

// at getNodeForPath we also check the path for isForbiddenFileOrDir
// with that we have covered both source and destination
$sourceNode = $this->getNodeForPath($sourcePath);
if ($sourceNode instanceof \Sabre\DAV\ICollection && $targetNodeExists) {
throw new \Sabre\DAV\Exception\Forbidden('Could not copy directory ' . $sourceNode->getName() . ', target exists');
Expand Down Expand Up @@ -316,7 +331,13 @@ public function copy($source, $destination) {
throw new Forbidden('No permissions to copy object.');
}

// this will trigger existence check
# check the destination path, for source see below
if (\OC\Files\Filesystem::isForbiddenFileOrDir($destination)) {
throw new \Sabre\DAV\Exception\Forbidden();
}

// at getNodeForPath we also check the path for isForbiddenFileOrDir
// with that we have covered both source and destination
$this->getNodeForPath($source);

list($destinationDir, $destinationName) = \Sabre\HTTP\URLUtil::splitPath($destination);
Expand Down
14 changes: 14 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,20 @@
*/
'blacklisted_files' => array('.htaccess'),

/**
* Exclude specific directory names and disallow scanning, creating and renaming
* using these names. Case insensitive.
* Excluded directory names are queried at any path part like at the beginning,
* in the middle or at the end and will not be further processed if found.
* Please see the documentation for details and examples.
* Use when the storage backend supports eg snapshot directories to be excluded.
* WARNING: USE THIS ONLY IF YOU KNOW WHAT YOU ARE DOING.
*/
'excluded_directories' =>
array (
'.snapshot',
'~snapshot',
),
/**
* Define a default folder for shared files and folders other than root.
*/
Expand Down
2 changes: 1 addition & 1 deletion core/Controller/AvatarController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public function postAvatar($path) {
if (
$files['error'][0] === 0 &&
is_uploaded_file($files['tmp_name'][0]) &&
!\OC\Files\Filesystem::isFileBlacklisted($files['tmp_name'][0])
!\OC\Files\Filesystem::isForbiddenFileOrDir($files['tmp_name'][0])
) {
if ($files['size'][0] > 20*1024*1024) {
return new JSONResponse(
Expand Down
4 changes: 2 additions & 2 deletions lib/base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,8 +874,8 @@ public static function registerLogRotate() {
*/
public static function registerFilesystemHooks() {
// Check for blacklisted files
OC_Hook::connect('OC_Filesystem', 'write', 'OC\Files\Filesystem', 'isBlacklisted');
OC_Hook::connect('OC_Filesystem', 'rename', 'OC\Files\Filesystem', 'isBlacklisted');
OC_Hook::connect('OC_Filesystem', 'write', 'OC\Files\Filesystem', 'isForbiddenFileOrDir_Hook');
OC_Hook::connect('OC_Filesystem', 'rename', 'OC\Files\Filesystem', 'isForbiddenFileOrDir_Hook');
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Files/Cache/Scanner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ protected function getNewChildren($folder) {
if ($dh = $this->storage->opendir($folder)) {
if (is_resource($dh)) {
while (($file = readdir($dh)) !== false) {
if (!Filesystem::isIgnoredDir($file)) {
if (!Filesystem::isIgnoredDir($file) && !Filesystem::isForbiddenFileOrDir($file)) {
$children[] = trim(\OC\Files\Filesystem::normalizePath($file), '/');
}
}
Expand Down
61 changes: 53 additions & 8 deletions lib/private/Files/Filesystem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,29 +565,26 @@ static public function isValidPath($path) {
*
* @param array $data from hook
*/
static public function isBlacklisted($data) {
static public function isForbiddenFileOrDir_Hook($data) {

@nickvergessen nickvergessen Oct 5, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method might be used by other apps?

@mmattel mmattel Oct 5, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when I originally created the PR in owncloud, I have found one app (but I do not remember anymore...) that was using this call. Therefore I left it and forwarded the call to the new function.

if (isset($data['path'])) {
$path = $data['path'];
} else if (isset($data['newpath'])) {
$path = $data['newpath'];
}
if (isset($path)) {
if (self::isFileBlacklisted($path)) {
if (self::isForbiddenFileOrDir($path)) {
$data['run'] = false;
}
}
}

/**
* depriciated, replaced by isForbiddenFileOrDir
* @param string $filename
* @return bool
* @return boolean
*/
static public function isFileBlacklisted($filename) {
$filename = self::normalizePath($filename);

$blacklist = \OC::$server->getConfig()->getSystemValue('blacklisted_files', array('.htaccess'));
$filename = strtolower(basename($filename));
return in_array($filename, $blacklist);
return self::isForbiddenFileOrDir($filename);
}

/**
Expand All @@ -604,6 +601,54 @@ static public function isIgnoredDir($dir) {
return false;
}

/**
* Check if the directory path / file name contains a Blacklisted or Excluded name
* config.php parameter arrays can contain file names to be blacklisted or directory names to be excluded
* Blacklist ... files that may harm the owncloud environment like a foreign .htaccess file
* Excluded ... directories that are excluded from beeing further processed, like snapshot directories
* The parameter $ed is only used in conjunction with unit tests as we handover here the excluded
* directory name to be tested against. $ed and the query with can be redesigned if filesystem.php will get
* a constructor where it is then possible to define the excluded directory names for unit tests.
* @param string $FileOrDir
* @param array $ed
* @return boolean
*/
static public function isForbiddenFileOrDir($FileOrDir, $ed = array()) {
$excluded = array();
$blacklist = array();
$path_parts = array();
$ppx = array();
$blacklist = \OC::$server->getSystemConfig()->getValue('blacklisted_files', array('.htaccess'));
if ($ed) {
$excluded = $ed;
} else {
$excluded = \OC::$server->getSystemConfig()->getValue('excluded_directories', $ed);
}
// explode '/'
$ppx = array_filter(explode('/', $FileOrDir), 'strlen');
$ppx = array_map('strtolower', $ppx);
// further explode each array element with '\' and add to result array if found
foreach($ppx as $pp) {
// only add an array element if strlen != 0
$path_parts = array_merge($path_parts, array_filter(explode('\\', $pp), 'strlen'));
}
if ($excluded) {
$excluded = array_map('trim', $excluded);
$excluded = array_map('strtolower', $excluded);
$match = array_intersect($path_parts, $excluded);
if ($match) {
return true;
}
}
$blacklist = array_map('trim', $blacklist);
$blacklist = array_map('strtolower', $blacklist);
$match = array_intersect($path_parts, $blacklist);
if ($match) {
return true;
}
return false;
}

/**
* following functions are equivalent to their php builtin equivalents for arguments/return values.
*/
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Files/Storage/Common.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public function copy($path1, $path2) {
$dir = $this->opendir($path1);
$this->mkdir($path2);
while ($file = readdir($dir)) {
if (!Filesystem::isIgnoredDir($file)) {
if (!Filesystem::isIgnoredDir($file) && !Filesystem::isForbiddenFileOrDir($file)) {
if (!$this->copy($path1 . '/' . $file, $path2 . '/' . $file)) {
return false;
}
Expand Down Expand Up @@ -557,7 +557,7 @@ public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceIntern
$result = $this->mkdir($targetInternalPath);
if (is_resource($dh)) {
while ($result and ($file = readdir($dh)) !== false) {
if (!Filesystem::isIgnoredDir($file)) {
if (!Filesystem::isIgnoredDir($file) && !Filesystem::isForbiddenFileOrDir($file)) {
$result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file);
}
}
Expand Down
14 changes: 9 additions & 5 deletions lib/private/Files/View.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ public function file_put_contents($path, $data) {
if (is_resource($data)) { //not having to deal with streams in file_put_contents makes life easier
$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
if (Filesystem::isValidPath($path)
and !Filesystem::isFileBlacklisted($path)
and !Filesystem::isForbiddenFileOrDir($path)
) {
$path = $this->getRelativePath($absolutePath);

Expand Down Expand Up @@ -722,7 +722,7 @@ public function rename($path1, $path2) {
if (
Filesystem::isValidPath($path2)
and Filesystem::isValidPath($path1)
and !Filesystem::isFileBlacklisted($path2)
and !Filesystem::isForbiddenFileOrDir($path2)
) {
$path1 = $this->getRelativePath($absolutePath1);
$path2 = $this->getRelativePath($absolutePath2);
Expand Down Expand Up @@ -843,7 +843,7 @@ public function copy($path1, $path2, $preserveMtime = false) {
if (
Filesystem::isValidPath($path2)
and Filesystem::isValidPath($path1)
and !Filesystem::isFileBlacklisted($path2)
and !Filesystem::isForbiddenFileOrDir($path2)
) {
$path1 = $this->getRelativePath($absolutePath1);
$path2 = $this->getRelativePath($absolutePath2);
Expand Down Expand Up @@ -1094,7 +1094,7 @@ private function basicOperation($operation, $path, $hooks = [], $extraParam = nu
$postFix = (substr($path, -1, 1) === '/') ? '/' : '';
$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
if (Filesystem::isValidPath($path)
and !Filesystem::isFileBlacklisted($path)
and !Filesystem::isForbiddenFileOrDir($path)
) {
$path = $this->getRelativePath($absolutePath);
if ($path == null) {
Expand Down Expand Up @@ -1408,13 +1408,17 @@ public function getDirectoryContent($directory, $mimetype_filter = '') {
/**
* @var \OC\Files\FileInfo[] $files
*/
$files = array_filter($contents, function(ICacheEntry $content) {
return (!\OC\Files\Filesystem::isForbiddenFileOrDir($content['path']));
});

$files = array_map(function (ICacheEntry $content) use ($path, $storage, $mount, $sharingDisabled) {
if ($sharingDisabled) {
$content['permissions'] = $content['permissions'] & ~\OCP\Constants::PERMISSION_SHARE;
}
$owner = $this->getUserObjectForOwner($storage->getOwner($content['path']));
return new FileInfo($path . '/' . $content['name'], $storage, $content['path'], $content, $mount, $owner);
}, $contents);
}, $files);

//add a folder for any mountpoint in this directory and add the sizes of other mountpoints to the folders
$mounts = Filesystem::getMountManager()->findIn($path);
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Security/CertificateManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public function createCertificateBundle() {
* @throws \Exception If the certificate could not get added
*/
public function addCertificate($certificate, $name) {
if (!Filesystem::isValidPath($name) or Filesystem::isFileBlacklisted($name)) {
if (!Filesystem::isValidPath($name) or Filesystem::isForbiddenFileOrDir($name)) {
throw new \Exception('Filename is not valid');
}

Expand Down
2 changes: 1 addition & 1 deletion lib/private/legacy/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ static function copyr($src, $dest) {
self::copyr("$src/$file", "$dest/$file");
}
}
} elseif (file_exists($src) && !\OC\Files\Filesystem::isFileBlacklisted($src)) {
} elseif (file_exists($src) && !\OC\Files\Filesystem::isForbiddenFileOrDir($src)) {
copy($src, $dest);
}
}
Expand Down
27 changes: 25 additions & 2 deletions tests/lib/Files/FilesystemTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public function isFileBlacklistedData() {
array('.htaccess\\', true),
array('/etc/foo\bar/.htaccess\\', true),
array('/etc/foo\bar/.htaccess/', true),
array('/etc/foo\bar/.htaccess/foo', false),
array('/etc/foo\bar/.htaccess/foo', true),
array('//foo//bar/\.htaccess/', true),
array('\foo\bar\.HTAccess', true),
);
Expand All @@ -269,7 +269,30 @@ public function isFileBlacklistedData() {
* @dataProvider isFileBlacklistedData
*/
public function testIsFileBlacklisted($path, $expected) {
$this->assertSame($expected, \OC\Files\Filesystem::isFileBlacklisted($path));
$this->assertSame($expected, \OC\Files\Filesystem::isForbiddenFileOrDir($path));
}

public function isExcludedData() {
return array(
array('.snapshot', true),
array('.snapshot/', true),
array('.snapshot\\', true),
array('/etc/foo/bar/foo.txt', false),
array('/.snapshot/etc/foo/bar/foo.txt', true),
array('/.snapShot/etc/foo/bar/foo.txt', true),
array('\.snapshot\etc\foo/bar\foo.txt', true),
array('/etc/foo/.snapshot', true),
array('/etc/foo/.snapshot/bar', true),
);
}

/**
* The parameter array can be redesigned if filesystem.php will get a constructor where it is possible to
* define the excluded directories for unit tests
* @dataProvider isExcludedData
*/
public function testIsExcluded($path, $expected) {
$this->assertSame($expected, \OC\Files\Filesystem::isForbiddenFileOrDir($path, array('.snapshot')));
}

public function testNormalizePathUTF8() {
Expand Down