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

Signed-off-by: Robin Appelman <[email protected]>
nextcloud · Nov 28, 2024 · 4d65a9f · 4d65a9f
1 parent dd101dd
commit 4d65a9f
Showing 1 changed file with 40 additions and 51 deletions.
diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
@@ -24,6 +24,7 @@
 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;
@@ -32,8 +33,6 @@
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
 use OCP\Server;
-use OCP\Share\IManager;
-use OCP\Share\IShare;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -701,6 +700,9 @@ public function rename($source, $target) {
 			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;
@@ -758,22 +760,20 @@ public function rename($source, $target) {
 					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);
+
+							/**
+							 * @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 ($storage1) {
@@ -783,6 +783,7 @@ public function rename($source, $target) {
 							}
 							// moving a file/folder between storages (from $storage1 to $storage2)
 						} else {
+							$this->validateMountMove($movedMounts, $mount1, $mount2);
 							$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
 						}
 
@@ -832,6 +833,28 @@ public function rename($source, $target) {
 		return $result;
 	}
 
+	private function validateMountMove(array $mounts, IMountPoint $sourceMount, IMountPoint $targetMount): void {
+		$targetType = 'storage';
+		if ($targetMount instanceof SharedMount) {
+			$targetType = 'share';
+		}
+		$targetPath = rtrim($targetMount->getMountPoint(), '/');
+
+		if ($sourceMount !== $targetMount) {
+			foreach ($mounts as $mount) {
+				$sourcePath = rtrim($mount->getMountPoint(), '/');
+				if (!$mount instanceof MoveableMount) {
+					throw new ForbiddenException("Storage {$sourcePath} cannot be moved", false);
+				}
+				$sourceType = 'storage';
+				if ($mount instanceof SharedMount) {
+					$sourceType = 'share';
+				}
+				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
 	 *
@@ -1791,40 +1814,6 @@ private function assertPathLength($path): void {
 		}
 	}
 
-	/**
-	 * check if it is allowed to move a mount point to a given target.
-	 * It is not allowed to move a mount point into a different mount point or
-	 * into an already shared folder
-	 */
-	private function targetIsNotShared(string $user, string $targetPath): bool {
-		$providers = [
-			IShare::TYPE_USER,
-			IShare::TYPE_GROUP,
-			IShare::TYPE_EMAIL,
-			IShare::TYPE_CIRCLE,
-			IShare::TYPE_ROOM,
-			IShare::TYPE_DECK,
-			IShare::TYPE_SCIENCEMESH
-		];
-		$shareManager = Server::get(IManager::class);
-		/** @var IShare[] $shares */
-		$shares = array_merge(...array_map(function (int $type) use ($shareManager, $user) {
-			return $shareManager->getSharesBy($user, $type);
-		}, $providers));
-
-		foreach ($shares as $share) {
-			$sharedPath = $share->getNode()->getPath();
-			if ($targetPath === $sharedPath || str_starts_with($targetPath, $sharedPath . '/')) {
-				$this->logger->debug(
-					'It is not allowed to move one mount point into a shared folder',
-					['app' => 'files']);
-				return false;
-			}
-		}
-
-		return true;
-	}
-
 	/**
 	 * Get a fileinfo object for files that are ignored in the cache (part files)
 	 */