Merge pull request #47756 from nextcloud/fixingPathShareCheck

fix(files): Check if target path is a descendant of the shared folder
nextcloud · Sep 6, 2024 · 141fed8 · 141fed8
2 parents e4fa996 + 0308001
commit 141fed8
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 2 deletions.
diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
@@ -1794,7 +1794,8 @@ private function targetIsNotShared(string $user, string $targetPath): bool {
 		}, $providers));
 
 		foreach ($shares as $share) {
-			if (str_starts_with($targetPath, $share->getNode()->getPath())) {
+			$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']);

diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php
@@ -1668,17 +1668,24 @@ public function testMoveMountPointIntoAnother() {
 	public function testMoveMountPointIntoSharedFolder() {
 		self::loginAsUser($this->user);
 
-		[$mount1] = $this->createTestMovableMountPoints([
+		[$mount1, $mount2] = $this->createTestMovableMountPoints([
 			$this->user . '/files/mount1',
+			$this->user . '/files/mount2',
 		]);
 
 		$mount1->expects($this->never())
 			->method('moveMount');
 
+		$mount2->expects($this->once())
+			->method('moveMount')
+			->willReturn(true);
+
 		$view = new View('/' . $this->user . '/files/');
 		$view->mkdir('shareddir');
 		$view->mkdir('shareddir/sub');
 		$view->mkdir('shareddir/sub2');
+		// Create a similar named but non-shared folder
+		$view->mkdir('shareddir notshared');
 
 		$fileId = $view->getFileInfo('shareddir')->getId();
 		$userObject = \OC::$server->getUserManager()->createUser('test2', 'IHateNonMockableStaticClasses');
@@ -1697,6 +1704,7 @@ public function testMoveMountPointIntoSharedFolder() {
 		$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');
+		$this->assertTrue($view->rename('mount2', 'shareddir notshared/sub'), 'Can move mount point into a similarly named but non-shared folder');
 
 		$shareManager->deleteShare($share);
 		$userObject->delete();