-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
fix(files): skip directory symlinks that loop back onto the scanned path #63868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -539,6 +539,73 @@ public function hasUpdated(string $path, int $time): bool { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Skips directory symlinks that resolve to the listed directory or one of | ||
| * its ancestors: following such a link sends the scanner back into the tree | ||
| * it is already walking, until the path length limit. Only the listing is | ||
| * filtered; resolving a path still follows the link. | ||
| */ | ||
| #[\Override] | ||
| public function getDirectoryContent(string $directory): \Traversable { | ||
| $ancestors = null; | ||
| foreach (parent::getDirectoryContent($directory) as $metadata) { | ||
| if ($metadata['mimetype'] === FileInfo::MIMETYPE_FOLDER) { | ||
| try { | ||
| $childSource = $this->getSourcePath(rtrim($directory, '/') . '/' . $metadata['name']); | ||
| } catch (ForbiddenException) { | ||
| // Retargeted outside the datadir since listed; drop it like getMetaData() would. | ||
| continue; | ||
| } | ||
| if (is_link($childSource)) { | ||
| $childReal = realpath($childSource); | ||
| if ($childReal !== false) { | ||
| // Built lazily, only once a symlinked directory shows up. | ||
| $ancestors ??= $this->getAncestorRealPaths($directory); | ||
| if (isset($ancestors[rtrim($childReal, '/')])) { | ||
| Server::get(LoggerInterface::class)->warning( | ||
| "Skipping looping directory symlink '$childSource' -> '$childReal'", | ||
| ['app' => 'core'] | ||
| ); | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| yield $metadata; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolved paths of $directory and each of its ancestors up to the storage | ||
| * root, as a set. A directory symlink resolving to any of them closes a loop. | ||
| */ | ||
| private function getAncestorRealPaths(string $directory): array { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this is not overcomplicated. I think it should be enough to check that the child real path doesn't match any of the parent paths, so splitting the path and then building every parent would be enough (without accessing the filesystem, just with string manipulation). |
||
| $root = rtrim($this->realDataDir, '/'); | ||
| $paths = []; | ||
| try { | ||
| $current = $this->getSourcePath(rtrim($directory, '/')); | ||
| } catch (ForbiddenException) { | ||
| // No resolvable ancestor chain: filter nothing. | ||
| return $paths; | ||
| } | ||
| while (true) { | ||
| $real = realpath($current); | ||
| if ($real !== false) { | ||
| $real = rtrim($real, '/'); | ||
| $paths[$real] = true; | ||
| if ($real === $root) { | ||
| break; | ||
| } | ||
| } | ||
| $parent = dirname($current); | ||
| if ($parent === $current || strlen($parent) < strlen($root)) { | ||
| break; | ||
| } | ||
| $current = $parent; | ||
| } | ||
| return $paths; | ||
| } | ||
|
|
||
| /** | ||
| * Get the source path (on disk) of a given path | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please invert the if statements and make them early-continues. That will be much more readable.