From eef257406730a989ec8e7a056c3d4234300fdb25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 21 Feb 2023 11:21:17 +0100 Subject: [PATCH] fix(core): Do not allow arbitrary path traversal in BinaryDataManager (#5523) --- packages/cli/src/Server.ts | 34 +++++++++++-------- .../core/src/BinaryDataManager/FileSystem.ts | 27 +++++++++------ packages/core/src/errors.ts | 5 +++ packages/core/src/index.ts | 1 + 4 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 packages/core/src/errors.ts diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index a5832346b..c90835b28 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -33,6 +33,7 @@ import { LoadNodeParameterOptions, LoadNodeListSearch, UserSettings, + FileNotFoundError, } from 'n8n-core'; import type { @@ -1149,21 +1150,26 @@ class Server extends AbstractServer { // TODO UM: check if this needs permission check for UM const identifier = req.params.path; const binaryDataManager = BinaryDataManager.getInstance(); - const binaryPath = binaryDataManager.getBinaryPath(identifier); - let { mode, fileName, mimeType } = req.query; - if (!fileName || !mimeType) { - try { - const metadata = await binaryDataManager.getBinaryMetadata(identifier); - fileName = metadata.fileName; - mimeType = metadata.mimeType; - res.setHeader('Content-Length', metadata.fileSize); - } catch {} + try { + const binaryPath = binaryDataManager.getBinaryPath(identifier); + let { mode, fileName, mimeType } = req.query; + if (!fileName || !mimeType) { + try { + const metadata = await binaryDataManager.getBinaryMetadata(identifier); + fileName = metadata.fileName; + mimeType = metadata.mimeType; + res.setHeader('Content-Length', metadata.fileSize); + } catch {} + } + if (mimeType) res.setHeader('Content-Type', mimeType); + if (mode === 'download') { + res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`); + } + res.sendFile(binaryPath); + } catch (error) { + if (error instanceof FileNotFoundError) res.writeHead(404).end(); + else throw error; } - if (mimeType) res.setHeader('Content-Type', mimeType); - if (mode === 'download') { - res.setHeader('Content-Disposition', `attachment; filename="${fileName}"`); - } - res.sendFile(binaryPath); }, ); diff --git a/packages/core/src/BinaryDataManager/FileSystem.ts b/packages/core/src/BinaryDataManager/FileSystem.ts index b66093b3d..df0335697 100644 --- a/packages/core/src/BinaryDataManager/FileSystem.ts +++ b/packages/core/src/BinaryDataManager/FileSystem.ts @@ -7,6 +7,7 @@ import type { BinaryMetadata } from 'n8n-workflow'; import { jsonParse } from 'n8n-workflow'; import type { IBinaryDataConfig, IBinaryDataManager } from '../Interfaces'; +import { FileNotFoundError } from '../errors'; const PREFIX_METAFILE = 'binarymeta'; const PREFIX_PERSISTED_METAFILE = 'persistedmeta'; @@ -85,17 +86,17 @@ export class BinaryDataFileSystem implements IBinaryDataManager { } getBinaryPath(identifier: string): string { - return path.join(this.storagePath, identifier); + return this.resolveStoragePath(identifier); } getMetadataPath(identifier: string): string { - return path.join(this.storagePath, `${identifier}.metadata`); + return this.resolveStoragePath(`${identifier}.metadata`); } async markDataForDeletionByExecutionId(executionId: string): Promise { const tt = new Date(new Date().getTime() + this.binaryDataTTL * 60000); return fs.writeFile( - path.join(this.getBinaryDataMetaPath(), `${PREFIX_METAFILE}_${executionId}_${tt.valueOf()}`), + this.resolveStoragePath('meta', `${PREFIX_METAFILE}_${executionId}_${tt.valueOf()}`), '', ); } @@ -116,8 +117,8 @@ export class BinaryDataFileSystem implements IBinaryDataManager { const timeAtNextHour = currentTime + 3600000 - (currentTime % 3600000); const timeoutTime = timeAtNextHour + this.persistedBinaryDataTTL * 60000; - const filePath = path.join( - this.getBinaryDataPersistMetaPath(), + const filePath = this.resolveStoragePath( + 'persistMeta', `${PREFIX_PERSISTED_METAFILE}_${executionId}_${timeoutTime}`, ); @@ -170,21 +171,18 @@ export class BinaryDataFileSystem implements IBinaryDataManager { const newBinaryDataId = this.generateFileName(prefix); return fs - .copyFile( - path.join(this.storagePath, binaryDataId), - path.join(this.storagePath, newBinaryDataId), - ) + .copyFile(this.resolveStoragePath(binaryDataId), this.resolveStoragePath(newBinaryDataId)) .then(() => newBinaryDataId); } async deleteBinaryDataByExecutionId(executionId: string): Promise { const regex = new RegExp(`${executionId}_*`); - const filenames = await fs.readdir(path.join(this.storagePath)); + const filenames = await fs.readdir(this.storagePath); const proms = filenames.reduce( (allProms, filename) => { if (regex.test(filename)) { - allProms.push(fs.rm(path.join(this.storagePath, filename))); + allProms.push(fs.rm(this.resolveStoragePath(filename))); } return allProms; @@ -253,4 +251,11 @@ export class BinaryDataFileSystem implements IBinaryDataManager { throw new Error(`Error finding file: ${filePath}`); } } + + private resolveStoragePath(...args: string[]) { + const returnPath = path.join(this.storagePath, ...args); + if (path.relative(this.storagePath, returnPath).startsWith('..')) + throw new FileNotFoundError('Invalid path detected'); + return returnPath; + } } diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts new file mode 100644 index 000000000..c425675c8 --- /dev/null +++ b/packages/core/src/errors.ts @@ -0,0 +1,5 @@ +export class FileNotFoundError extends Error { + constructor(readonly filePath: string) { + super(`File not found: ${filePath}`); + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cef794cfa..7a77667f5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -15,6 +15,7 @@ export * from './LoadNodeListSearch'; export * from './NodeExecuteFunctions'; export * from './WorkflowExecute'; export { eventEmitter, NodeExecuteFunctions, UserSettings }; +export * from './errors'; declare module 'http' { export interface IncomingMessage {