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).
This commit is contained in:
@@ -12,7 +12,24 @@ const { rimraf } = require('rimraf');
|
|||||||
|
|
||||||
// const PATH_SEGMENT_TO_URL_REPLACE_REGEX = /(public|private)\//;
|
// 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 {
|
class LocalFileManager {
|
||||||
// eslint-disable-next-line class-methods-use-this
|
// eslint-disable-next-line class-methods-use-this
|
||||||
@@ -41,14 +58,25 @@ class LocalFileManager {
|
|||||||
async read(filePathSegment, { withHeaders = false } = {}) {
|
async read(filePathSegment, { withHeaders = false } = {}) {
|
||||||
const filePath = buildPath(filePathSegment);
|
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;
|
let stat;
|
||||||
try {
|
try {
|
||||||
stat = await fs.promises.stat(filePath);
|
stat = await fs.promises.stat(realFilePath);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
throw new Error('File does not exist');
|
throw new Error('File does not exist');
|
||||||
}
|
}
|
||||||
|
|
||||||
const readStream = fs.createReadStream(filePath);
|
const readStream = fs.createReadStream(realFilePath);
|
||||||
|
|
||||||
if (withHeaders) {
|
if (withHeaders) {
|
||||||
return [
|
return [
|
||||||
|
|||||||
Reference in New Issue
Block a user