From 9d3f5c310d59d83fc340a3aea561502e2d991b6a Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 3 Jan 2026 13:04:32 -0500 Subject: [PATCH 1/2] refactor(filesystem): clean up isValidPath() logic and update security docs Signed-off-by: Josh --- lib/private/Files/Filesystem.php | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index eb374d9326b3d..372b69d07acda 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -390,17 +390,25 @@ public static function mount($class, $arguments, $mountpoint) { } /** - * check if the requested path is valid + * Validates a file path for safety in the Nextcloud virtual filesystem. * - * @param string $path - * @return bool + * This method is the primary defense against directory traversal and + * path manipulation attacks. It ALWAYS normalizes the input (see {@see normalizePath}), + * converting slash and unicode variants to a canonical absolute path. + * + * After normalization, any path containing '/..' segments (traversal attempts) is rejected. + * Normalized paths like '//foo//bar' are allowed and become '/foo/bar'. + * + * @param string $path Untrusted or user-supplied path to check. + * @return bool True if the path is safe, false otherwise. */ - public static function isValidPath($path) { + public static function isValidPath(string $path): bool { $path = self::normalizePath($path); - if (!$path || $path[0] !== '/') { - $path = '/' . $path; - } - if (str_contains($path, '/../') || strrchr($path, '/') === '/..') { + + // Invariant: normalized path is non-empty and absolute. + assert($path !== '' && $path[0] === '/', 'normalizePath() must return absolute path'); + + if (str_contains($path, '/../') || str_ends_with($path, '/..')) { return false; } return true; From ddd7dc63c7062be0238675577a8995396eaca87b Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 3 Jan 2026 13:31:23 -0500 Subject: [PATCH 2/2] test(filesystem): Add empty path to isValidPathData Signed-off-by: Josh --- tests/lib/Files/FilesystemTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/Files/FilesystemTest.php b/tests/lib/Files/FilesystemTest.php index b838310d28b83..6f7b9f91fcc63 100644 --- a/tests/lib/Files/FilesystemTest.php +++ b/tests/lib/Files/FilesystemTest.php @@ -234,6 +234,7 @@ public function testNormalizePathKeepUnicodeCache(): void { public static function isValidPathData(): array { return [ ['/', true], + ['', true], ['/path', true], ['/foo/bar', true], ['/foo//bar/', true],