From bd487e3f9f15948dee9427cc0ddbe891cdba6da0 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Mon, 10 Aug 2026 03:00:29 +0200 Subject: [PATCH] fix: Prevent path traversal in local file manager The local file manager joined attacker-controlled path segments into the uploads storage path without ensuring the result stayed under the uploads root. Any authenticated user could read arbitrary files readable by the Planka process (e.g. /etc/passwd, /app/.env) via `../` sequences on the /user-avatars/*, /background-images/* and /favicons/* routes (CWE-22). Resolve the path and enforce it stays within uploadsBasePath, centrally in buildPath so every local file operation is covered. Additionally resolve symlinks in read() and re-check containment, so a symlink inside the root cannot be used to escape it. Reported by Alpesh (github.com/Alpastx). --- .../hooks/file-manager/LocalFileManager.js | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/server/api/hooks/file-manager/LocalFileManager.js b/server/api/hooks/file-manager/LocalFileManager.js index f9da1a6e..6fdbc419 100644 --- a/server/api/hooks/file-manager/LocalFileManager.js +++ b/server/api/hooks/file-manager/LocalFileManager.js @@ -12,7 +12,24 @@ const { rimraf } = require('rimraf'); // const PATH_SEGMENT_TO_URL_REPLACE_REGEX = /(public|private)\//; -const buildPath = (pathSegment) => path.join(sails.config.custom.uploadsBasePath, pathSegment); +// Reject a path unless it is the uploads root itself or lives strictly +// beneath it. Callers must pass an already-absolute path. +const assertWithinRoot = (rootPath, filePath) => { + if (filePath !== rootPath && !filePath.startsWith(`${rootPath}${path.sep}`)) { + throw new Error('Path is outside of the uploads directory'); + } +}; + +const buildPath = (pathSegment) => { + const { uploadsBasePath } = sails.config.custom; + const filePath = path.resolve(uploadsBasePath, pathSegment); + + // Ensure the resolved path stays within the uploads root, so that + // attacker-controlled path segments (e.g. `../`) cannot escape it. + assertWithinRoot(uploadsBasePath, filePath); + + return filePath; +}; class LocalFileManager { // eslint-disable-next-line class-methods-use-this @@ -41,14 +58,25 @@ class LocalFileManager { async read(filePathSegment, { withHeaders = false } = {}) { const filePath = buildPath(filePathSegment); + // Resolve symlinks and re-check containment, so that a symlink placed + // inside the uploads root cannot be used to read files outside of it. + let realFilePath; + try { + realFilePath = await fs.promises.realpath(filePath); + } catch (error) { + throw new Error('File does not exist'); + } + const realBasePath = await fs.promises.realpath(sails.config.custom.uploadsBasePath); + assertWithinRoot(realBasePath, realFilePath); + let stat; try { - stat = await fs.promises.stat(filePath); + stat = await fs.promises.stat(realFilePath); } catch (error) { throw new Error('File does not exist'); } - const readStream = fs.createReadStream(filePath); + const readStream = fs.createReadStream(realFilePath); if (withHeaders) { return [