Skip to content

Commit ea4a989

Browse files
nikaiwnikaiw
committed
fix(files): skip directory symlinks that loop back onto the scanned path
SUMMARY On a local storage, symlinks to a directories are followed even if the directory is a parent directory or the directory itself. Under these conditions, the Nextcloud metadata scanner continues traversing the tree until it reaches the limit. Re-entering the same tree in loop. Basically it will fill the oc_filecache SQL table with a new entry until the path reach the limit of oc_filecache.path column varchar(4000) Impact We had a server crash because Nextcloud filled the database disk by creating 30M row in this table and producing a 185 GB database. This is a DoS to become that can be triggered just by creating a symlink. History The bug is known since at least 2017 see #6395 (SMB), #20197, #23022 A previous fix #21723 was closed unmerged. Proposed Fix Local::getDirectoryContent() will skip a symlink whose target is the directory being listed or a parent directory. We are comparing both path after resolution so that we also catches cycles ( a -> b, b->a ). Signed-off-by: NK <nicolas.devillers@airbus.com> Assisted-by: ClaudeCode:claude-fable-5
1 parent 76dc4c7 commit ea4a989

3 files changed

Lines changed: 199 additions & 0 deletions

File tree

‎lib/private/Files/Storage/Local.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,73 @@ public function hasUpdated(string $path, int $time): bool {
539539
}
540540
}
541541

542+
/**
543+
* Skips directory symlinks that resolve to the listed directory or one of
544+
* its ancestors: following such a link sends the scanner back into the tree
545+
* it is already walking, until the path length limit. Only the listing is
546+
* filtered; resolving a path still follows the link.
547+
*/
548+
#[\Override]
549+
public function getDirectoryContent(string $directory): \Traversable {
550+
$ancestors = null;
551+
foreach (parent::getDirectoryContent($directory) as $metadata) {
552+
if ($metadata['mimetype'] === FileInfo::MIMETYPE_FOLDER) {
553+
try {
554+
$childSource = $this->getSourcePath(rtrim($directory, '/') . '/' . $metadata['name']);
555+
} catch (ForbiddenException) {
556+
// Retargeted outside the datadir since listed; drop it like getMetaData() would.
557+
continue;
558+
}
559+
if (is_link($childSource)) {
560+
$childReal = realpath($childSource);
561+
if ($childReal !== false) {
562+
// Built lazily, only once a symlinked directory shows up.
563+
$ancestors ??= $this->getAncestorRealPaths($directory);
564+
if (isset($ancestors[rtrim($childReal, '/')])) {
565+
Server::get(LoggerInterface::class)->warning(
566+
"Skipping looping directory symlink '$childSource' -> '$childReal'",
567+
['app' => 'core']
568+
);
569+
continue;
570+
}
571+
}
572+
}
573+
}
574+
yield $metadata;
575+
}
576+
}
577+
578+
/**
579+
* Resolved paths of $directory and each of its ancestors up to the storage
580+
* root, as a set. A directory symlink resolving to any of them closes a loop.
581+
*/
582+
private function getAncestorRealPaths(string $directory): array {
583+
$root = rtrim($this->realDataDir, '/');
584+
$paths = [];
585+
try {
586+
$current = $this->getSourcePath(rtrim($directory, '/'));
587+
} catch (ForbiddenException) {
588+
// No resolvable ancestor chain: filter nothing.
589+
return $paths;
590+
}
591+
while (true) {
592+
$real = realpath($current);
593+
if ($real !== false) {
594+
$real = rtrim($real, '/');
595+
$paths[$real] = true;
596+
if ($real === $root) {
597+
break;
598+
}
599+
}
600+
$parent = dirname($current);
601+
if ($parent === $current || strlen($parent) < strlen($root)) {
602+
break;
603+
}
604+
$current = $parent;
605+
}
606+
return $paths;
607+
}
608+
542609
/**
543610
* Get the source path (on disk) of a given path
544611
*

‎tests/lib/Files/Cache/ScannerTest.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,4 +452,40 @@ public function testNoETagUnscannedSubFolder(): void {
452452
$newFolderEntry2 = $this->cache->get('folder/sub');
453453
$this->assertNotEquals($newFolderEntry2->getEtag(), $oldFolderEntry2->getEtag());
454454
}
455+
public function testScanSkipsSelfReferencingSymlink(): void {
456+
$root = rtrim($this->storage->getSourcePath(''), '/');
457+
mkdir($root . '/dir');
458+
file_put_contents($root . '/dir/real.txt', 'data');
459+
symlink($root . '/dir', $root . '/dir/self');
460+
461+
$this->scanner->scan('');
462+
463+
$this->assertTrue($this->cache->inCache('dir'));
464+
$this->assertTrue($this->cache->inCache('dir/real.txt'));
465+
// the loop itself is skipped, at every level
466+
$this->assertFalse($this->cache->inCache('dir/self'));
467+
$this->assertFalse($this->cache->inCache('dir/self/real.txt'));
468+
$this->assertFalse($this->cache->inCache('dir/self/self'));
469+
}
470+
471+
public function testScanSkipsIndirectSymlinkCycle(): void {
472+
$root = rtrim($this->storage->getSourcePath(''), '/');
473+
mkdir($root . '/x');
474+
mkdir($root . '/y');
475+
symlink($root . '/y', $root . '/x/toy');
476+
symlink($root . '/x', $root . '/y/tox');
477+
478+
$this->scanner->scan('');
479+
480+
$this->assertTrue($this->cache->inCache('x'));
481+
$this->assertTrue($this->cache->inCache('y'));
482+
// a link to a sibling is legitimate and stays visible...
483+
$this->assertTrue($this->cache->inCache('x/toy'));
484+
$this->assertTrue($this->cache->inCache('y/tox'));
485+
// ...but the walk stops where the cycle closes: entering x/toy lands
486+
// in y, whose link back to x would re-enter the path being walked
487+
$this->assertFalse($this->cache->inCache('x/toy/tox'));
488+
$this->assertFalse($this->cache->inCache('y/tox/toy'));
489+
}
490+
455491
}

‎tests/lib/Files/Storage/LocalTest.php‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,100 @@ public function testFopenRecoversFromStaleRealpathCache(string $staleName, strin
280280
$this->assertSame('abc', stream_get_contents($handle));
281281
fclose($handle);
282282
}
283+
private function collectNames(\Traversable $content): array {
284+
$names = [];
285+
foreach ($content as $metadata) {
286+
$names[] = $metadata['name'];
287+
}
288+
sort($names);
289+
return $names;
290+
}
291+
292+
public function testGetDirectoryContentSkipsSelfReferencingSymlink(): void {
293+
mkdir($this->tmpDir . 'dir');
294+
mkdir($this->tmpDir . 'dir/real');
295+
symlink($this->tmpDir . 'dir', $this->tmpDir . 'dir/self');
296+
297+
$storage = new Local(['datadir' => $this->tmpDir]);
298+
299+
$this->assertEquals(['real'], $this->collectNames($storage->getDirectoryContent('dir')));
300+
}
301+
302+
public function testGetDirectoryContentSkipsParentSymlink(): void {
303+
mkdir($this->tmpDir . 'a');
304+
mkdir($this->tmpDir . 'a/b');
305+
mkdir($this->tmpDir . 'a/b/real');
306+
symlink($this->tmpDir . 'a', $this->tmpDir . 'a/b/up');
307+
308+
$storage = new Local(['datadir' => $this->tmpDir]);
309+
310+
$this->assertEquals(['real'], $this->collectNames($storage->getDirectoryContent('a/b')));
311+
}
312+
313+
/**
314+
* a -> b while b -> a: neither target is an ancestor of its own parent, so a
315+
* parent-only check walks the pair forever.
316+
*/
317+
public function testGetDirectoryContentSkipsIndirectCycle(): void {
318+
mkdir($this->tmpDir . 'x');
319+
mkdir($this->tmpDir . 'y');
320+
symlink($this->tmpDir . 'y', $this->tmpDir . 'x/toy');
321+
symlink($this->tmpDir . 'x', $this->tmpDir . 'y/tox');
322+
323+
$storage = new Local(['datadir' => $this->tmpDir]);
324+
325+
// Entering x/toy lands in y; y's link back to x closes the cycle.
326+
$this->assertEquals([], $this->collectNames($storage->getDirectoryContent('x/toy')));
327+
}
328+
329+
/**
330+
* The loops we care about sit under ancestors that are themselves symlinks,
331+
* so the logical path and the resolved path are on different branches.
332+
* Comparing a logical path against a resolved one misses exactly this.
333+
*/
334+
public function testGetDirectoryContentSkipsLoopReachedThroughASymlinkedAncestor(): void {
335+
mkdir($this->tmpDir . 'real');
336+
mkdir($this->tmpDir . 'real/leaf');
337+
symlink($this->tmpDir . 'real', $this->tmpDir . 'alias');
338+
symlink($this->tmpDir . 'real', $this->tmpDir . 'real/leaf/back');
339+
340+
$storage = new Local(['datadir' => $this->tmpDir]);
341+
342+
// Reached as alias/leaf, whose resolved parent is real/leaf: 'back'
343+
// resolves to 'real', an ancestor, even though the logical path says alias/.
344+
$this->assertEquals([], $this->collectNames($storage->getDirectoryContent('alias/leaf')));
345+
}
346+
347+
public function testGetDirectoryContentKeepsNonLoopingSymlinks(): void {
348+
mkdir($this->tmpDir . 'a');
349+
mkdir($this->tmpDir . 'other');
350+
mkdir($this->tmpDir . 'other/deep');
351+
file_put_contents($this->tmpDir . 'other/f.txt', 'x');
352+
symlink($this->tmpDir . 'other', $this->tmpDir . 'a/sibling');
353+
symlink($this->tmpDir . 'other/deep', $this->tmpDir . 'a/deeper');
354+
symlink($this->tmpDir . 'other/f.txt', $this->tmpDir . 'a/afile');
355+
356+
$storage = new Local(['datadir' => $this->tmpDir]);
357+
358+
$this->assertEquals(
359+
['afile', 'deeper', 'sibling'],
360+
$this->collectNames($storage->getDirectoryContent('a'))
361+
);
362+
}
363+
364+
/**
365+
* Enumeration is filtered, resolution is not: a file behind a directory
366+
* symlink stays reachable. Guards the behaviour asserted by
367+
* testDisallowSymlinksInsideDatadir.
368+
*/
369+
public function testLoopingSymlinkStillResolvesForFileAccess(): void {
370+
mkdir($this->tmpDir . 'dir');
371+
symlink($this->tmpDir . 'dir', $this->tmpDir . 'dir/self');
372+
373+
$storage = new Local(['datadir' => $this->tmpDir]);
374+
$storage->file_put_contents('dir/self/foo', 'bar');
375+
376+
$this->assertEquals('bar', $storage->file_get_contents('dir/foo'));
377+
}
378+
283379
}

0 commit comments

Comments
 (0)