upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-11 01:30:50 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 79

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

Browse source 
Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
Robin Appelman 2024-11-28 19:08:27 +01:00 committed by Andy Scherzinger
parent 6118649d54
commit 68829ad47c
No known key found for this signature in database
GPG key ID: 6CADC7E3523C308B
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
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;
@ -700,6 +701,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;
@ -757,24 +761,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 {
@ -782,6 +787,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);
}
@ -831,6 +839,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
View file

@ -20,6 +20,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;
@ -1639,10 +1640,7 @@ 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() {
public function testMoveMountPointOverwrite(): void {
self::loginAsUser($this->user);
[$mount1, $mount2] = $this->createTestMovableMountPoints([
@ -1658,8 +1656,28 @@ 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');
}
public function testMoveMountPointIntoMount(): 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/sub');
}
/**
@ -1701,9 +1719,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);