sascha
/
nextcloud-server
mirror of https://github.com/nextcloud/server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: improve checks for moving shares/storages into other mounts 

Signed-off-by: Robin Appelman <robin@icewind.nl>
pull/49552/head
Robin Appelman 2024-11-28 19:08:27 +07:00
parent b16f5a0043
commit 16d0363937
No known key found for this signature in database
GPG Key ID: 42B69D8A64526EFB
2 changed files with 93 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
lib/private/Files/View.php
Unescape Escape View File

@ -24,6 +24,7 @@ use OCP\Files\ForbiddenException;
use OCP\Files\InvalidCharacterInPathException;
use OCP\Files\InvalidDirectoryException;
use OCP\Files\InvalidPathException;
use OCP\Files\Mount\IMountManager;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\NotFoundException;
use OCP\Files\ReservedWordException;
@ -707,6 +708,9 @@ class View {
throw new ForbiddenException('Moving a folder into a child folder is forbidden', false);
}
/** @var IMountManager $mountManager */
$mountManager = \OC::$server->get(IMountManager::class);
$targetParts = explode('/', $absolutePath2);
$targetUser = $targetParts[1] ?? null;
$result = false;
@ -764,24 +768,25 @@ class View {
try {
$this->changeLock($target, ILockingProvider::LOCK_EXCLUSIVE, true);
$movedMounts = $mountManager->findIn($this->getAbsolutePath($source));
if ($internalPath1 === '') {
if ($mount1 instanceof MoveableMount) {
$sourceParentMount = $this->getMount(dirname($source));
if ($sourceParentMount === $mount2 && $this->targetIsNotShared($targetUser, $absolutePath2)) {
/**
* @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount1
*/
$sourceMountPoint = $mount1->getMountPoint();
$result = $mount1->moveMount($absolutePath2);
$manager->moveMount($sourceMountPoint, $mount1->getMountPoint());
} else {
$result = false;
}
} else {
$result = false;
}
$sourceParentMount = $this->getMount(dirname($source));
$movedMounts[] = $mount1;
$this->validateMountMove($movedMounts, $sourceParentMount, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2));
/**
* @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount1
*/
$sourceMountPoint = $mount1->getMountPoint();
$result = $mount1->moveMount($absolutePath2);
$manager->moveMount($sourceMountPoint, $mount1->getMountPoint());
// moving a file/folder within the same mount point
} elseif ($storage1 === $storage2) {
if (count($movedMounts) > 0) {
$this->validateMountMove($movedMounts, $mount1, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2));
}
if ($storage1) {
$result = $storage1->rename($internalPath1, $internalPath2);
} else {
@ -789,6 +794,9 @@ class View {
}
// moving a file/folder between storages (from $storage1 to $storage2)
} else {
if (count($movedMounts) > 0) {
$this->validateMountMove($movedMounts, $mount1, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2));
}
$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
}
@ -838,6 +846,34 @@ class View {
return $result;
}
private function validateMountMove(array $mounts, IMountPoint $sourceMount, IMountPoint $targetMount, bool $targetIsShared): void {
$targetType = 'storage';
if ($targetMount instanceof SharedMount) {
$targetType = 'share';
}
$targetPath = rtrim($targetMount->getMountPoint(), '/');
foreach ($mounts as $mount) {
$sourcePath = rtrim($mount->getMountPoint(), '/');
$sourceType = 'storage';
if ($mount instanceof SharedMount) {
$sourceType = 'share';
}
if (!$mount instanceof MoveableMount) {
throw new ForbiddenException("Storage {$sourcePath} cannot be moved", false);
}
if ($targetIsShared) {
throw new ForbiddenException("Moving a $sourceType ($sourcePath) into shared folder is not allowed", false);
}
if ($sourceMount !== $targetMount) {
throw new ForbiddenException("Moving a $sourceType ($sourcePath) into another $targetType ($targetPath) is not allowed", false);
}
}
}
/**
* Copy a file/folder from the source path to target path
*

51
tests/lib/Files/ViewTest.php
Unescape Escape View File

@ -21,6 +21,7 @@ use OCP\Cache\CappedMemoryCache;
use OCP\Constants;
use OCP\Files\Config\IMountProvider;
use OCP\Files\FileInfo;
use OCP\Files\ForbiddenException;
use OCP\Files\GenericFileException;
use OCP\Files\Mount\IMountManager;
use OCP\Files\Storage\IStorage;
@ -1641,10 +1642,27 @@ class ViewTest extends \Test\TestCase {
$this->assertTrue($view->rename('mount2', 'sub/moved_mount'), 'Can move a mount point into a subdirectory');
}
/**
* Test that moving a mount point into another is forbidden
*/
public function testMoveMountPointIntoAnother(): void {
public function testMoveMountPointOverwrite(): void {
self::loginAsUser($this->user);
[$mount1, $mount2] = $this->createTestMovableMountPoints([
$this->user . '/files/mount1',
$this->user . '/files/mount2',
]);
$mount1->expects($this->never())
->method('moveMount');
$mount2->expects($this->never())
->method('moveMount');
$view = new View('/' . $this->user . '/files/');
$this->expectException(ForbiddenException::class);
$view->rename('mount1', 'mount2');
}
public function testMoveMountPointIntoMount(): void {
self::loginAsUser($this->user);
[$mount1, $mount2] = $this->createTestMovableMountPoints([
@ -1660,8 +1678,8 @@ class ViewTest extends \Test\TestCase {
$view = new View('/' . $this->user . '/files/');
$this->assertFalse($view->rename('mount1', 'mount2'), 'Cannot overwrite another mount point');
$this->assertFalse($view->rename('mount1', 'mount2/sub'), 'Cannot move a mount point into another');
$this->expectException(ForbiddenException::class);
$view->rename('mount1', 'mount2/sub');
}
/**
@ -1703,9 +1721,24 @@ class ViewTest extends \Test\TestCase {
->setNode($shareDir);
$shareManager->createShare($share);
$this->assertFalse($view->rename('mount1', 'shareddir'), 'Cannot overwrite shared folder');
$this->assertFalse($view->rename('mount1', 'shareddir/sub'), 'Cannot move mount point into shared folder');
$this->assertFalse($view->rename('mount1', 'shareddir/sub/sub2'), 'Cannot move mount point into shared subfolder');
try {
$view->rename('mount1', 'shareddir');
$this->fail('Cannot overwrite shared folder');
} catch (ForbiddenException $e) {
}
try {
$view->rename('mount1', 'shareddir/sub');
$this->fail('Cannot move mount point into shared folder');
} catch (ForbiddenException $e) {
}
try {
$view->rename('mount1', 'shareddir/sub/sub2');
$this->fail('Cannot move mount point into shared subfolder');
} catch (ForbiddenException $e) {
}
$this->assertTrue($view->rename('mount2', 'shareddir notshared/sub'), 'Can move mount point into a similarly named but non-shared folder');
$shareManager->deleteShare($share);