Skip to content

fix(files): skip directory symlinks that loop back onto the scanned path - #63868

Open
nikaiw wants to merge 1 commit into
nextcloud:masterfrom
nikaiw:fix/scanner-symlink-loops
Open

fix(files): skip directory symlinks that loop back onto the scanned path#63868
nikaiw wants to merge 1 commit into
nextcloud:masterfrom
nikaiw:fix/scanner-symlink-loops

Conversation

@nikaiw

@nikaiw nikaiw commented Sep 1, 2026

Copy link
Copy Markdown

Summary

A directory symlink whose target is one of its own ancestors, e.g. device -> .. in a copied /sys tree or docker-ce's bundles/latest -> ., makes the scanner re-enter the same tree until the 4000 character path limit:

.../4:0:0:0/bsg/4:0:0:0/device/bsg/4:0:0:0/device/... (~140 levels)

Each level adds a filecache row, so scanning such a storage grows oc_filecache table without bound and the background scan never finishes.

In effect this is a DoS: anything able to write symlinks to the backing storage (a build job, rsync, sftp) can grow the database until the disk is full. On one instance two stale build trees produced 30M bogus rows and a 185 GB database, and at their peak filled the node's disk, taking the server down.

The bug has been open since 2017 (#6395, see also #20197 and #23022). The previous attempt, #21723, compared the logical path to the resolved one, which cannot match when an ancestor is itself a symlink, and was closed unmerged.

Skip a child symlink in Local::getDirectoryContent() when its resolved target equals the listed directory or one of its ancestors, comparing resolved paths on both sides.

This also catches indirect cycles (a -> b, b -> a). The loop above now stops where it closes, with a single log line instead of a runaway descent:

Skipping looping directory symlink '.../4:0:0:0/bsg/4:0:0:0/device'
-> '.../4:0:0:0'

Only listing is affected; files behind such links stay readable and writable (see testDisallowSymlinksInsideDatadir).

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

A directory symlink whose target is one of its own ancestors, e.g.
`device -> ..` in a copied /sys tree or docker-ce's
`bundles/latest -> .`, makes the scanner re-enter the same tree until
the 4000 character path limit:

  .../4:0:0:0/bsg/4:0:0:0/device/bsg/4:0:0:0/device/...  (~140 levels)

Each level adds a filecache row, so scanning such a storage grows
oc_filecache without bound and the background scan never finishes. In
effect this is a denial of service: anything able to write symlinks to
the backing storage (a build job, rsync, sftp) can grow the database
until the disk is full. On one instance two stale build trees produced
30M bogus rows and a 185 GB database, and at their peak filled the
node's disk, taking the server down.

The bug has been open since 2017 (nextcloud#6395, see also nextcloud#20197 and nextcloud#23022).
The previous attempt, nextcloud#21723, compared the logical path to the resolved
one, which cannot match when an ancestor is itself a symlink, and was
closed unmerged.

Skip a child symlink in Local::getDirectoryContent() when its resolved
target equals the listed directory or one of its ancestors, comparing
resolved paths on both sides. This also catches indirect cycles
(a -> b, b -> a). The loop above now stops where it closes, with a
single log line instead of a runaway descent:

  Skipping looping directory symlink '.../4:0:0:0/bsg/4:0:0:0/device'
  -> '.../4:0:0:0'

Only listing is affected; files behind such links stay readable and
writable (see testDisallowSymlinksInsideDatadir).

Signed-off-by: NK <nicolas.devillers@airbus.com>
@nikaiw
nikaiw requested a review from a team as a code owner September 1, 2026 00:14
@nikaiw
nikaiw requested review from Altahrim, icewind1991, leftybournes and sorbaugh and removed request for a team September 1, 2026 00:14
@joshtrichards

Copy link
Copy Markdown
Member

Also #41563

@joshtrichards joshtrichards added bug 3. to review Waiting for reviews hotspot: filename handling Filenames - invalid, portable, blacklisting, etc. community pull requests from community feature: filesystem labels Sep 1, 2026
@susnux

susnux commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution!

Please follow our AI policy: https://github.com/nextcloud/.github/blob/master/AI_POLICY.md#disclosure
You commit is missing the AI disclosure. Moreover communication has to be done by a human meaning please use your own words for the PR summary and commit message - it shows you reviewed and understood the AI output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews AI assisted bug community pull requests from community feature: filesystem hotspot: filename handling Filenames - invalid, portable, blacklisting, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: files:scan follows symlink -> endless loop

3 participants