From 351670fbaffa844e89829a4df45cf3a8df80688b 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, 4 Jul 2023 20:04:31 +0200 Subject: [PATCH] move all checks to NodeExecuteFunctions --- packages/core/src/NodeExecuteFunctions.ts | 31 +++++++++++++++++-- .../ReadBinaryFile/ReadBinaryFile.node.ts | 3 -- .../ReadBinaryFiles/ReadBinaryFiles.node.ts | 3 -- .../WriteBinaryFile/WriteBinaryFile.node.ts | 6 +--- packages/workflow/src/Interfaces.ts | 4 +-- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 16f8f7b691586..af70503282a3e 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -114,7 +114,7 @@ import type { import axios from 'axios'; import url, { URL, URLSearchParams } from 'url'; import { Readable } from 'stream'; -import { access as fsAccess } from 'fs/promises'; +import { access as fsAccess, writeFile as fsWriteFile } from 'fs/promises'; import { createReadStream } from 'fs'; import { BinaryDataManager } from './BinaryDataManager'; @@ -129,6 +129,7 @@ import { setAllWorkflowExecutionMetadata, setWorkflowExecutionMetadata, } from './WorkflowExecutionMetadata'; +import { getUserN8nFolderPath } from './UserSettings'; axios.defaults.timeout = 300000; // Prevent axios from adding x-form-www-urlencoded headers by default @@ -2251,6 +2252,18 @@ const getRequestHelperFunctions = ( }, }); +const restrictedPaths = (process.env.RESTRICT_FILE_ACCESS_TO ?? '') + .split(';') + .map((path) => path.trim()) + .filter((path) => path) + .concat(getUserN8nFolderPath()); +// TODO: add other folders here + +const isFilePathBlocked = (filePath: string) => { + const absolutePath = path.resolve(filePath); + return restrictedPaths.some((dir) => !path.relative(dir, absolutePath).includes('..')); +}; + const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => ({ async createReadStream(filePath) { try { @@ -2258,12 +2271,26 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => } catch (error) { throw error.code === 'ENOENT' ? new NodeOperationError(node, error, { - message: `The file "${String(filePath)}" could not be accessed.`, + message: `The file "${filePath}" could not be accessed.`, + severity: 'warning', }) : error; } + if (isFilePathBlocked(filePath)) { + throw new NodeOperationError(node, `The file "${String(filePath)}" could not be accessed.`, { + severity: 'warning', + }); + } return createReadStream(filePath); }, + async writeContentToFile(filePath, content, flag) { + if (isFilePathBlocked(filePath)) { + throw new NodeOperationError(node, `The file "${String(filePath)}" is not writable.`, { + severity: 'warning', + }); + } + return fsWriteFile(filePath, content, { encoding: 'binary', flag }); + }, }); const getNodeHelperFunctions = ({ diff --git a/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts b/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts index 1fe41c5cb8443..00f833f40663c 100644 --- a/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts +++ b/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts @@ -5,7 +5,6 @@ import type { INodeTypeDescription, } from 'n8n-workflow'; -import { checkFilePathAccess } from '@utils/utilities'; import { allowedPathsNotice } from '@utils/descriptions'; export class ReadBinaryFile implements INodeType { @@ -71,8 +70,6 @@ export class ReadBinaryFile implements INodeType { } const filePath = this.getNodeParameter('filePath', itemIndex); - checkFilePathAccess(filePath); - const stream = await this.helpers.createReadStream(filePath); const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex); newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(stream, filePath); diff --git a/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts b/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts index 720be0f57471f..825951a05855a 100644 --- a/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts +++ b/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts @@ -8,7 +8,6 @@ import { import glob from 'fast-glob'; -import { checkFilePathAccess } from '@utils/utilities'; import { allowedPathsNotice } from '@utils/descriptions'; export class ReadBinaryFiles implements INodeType { @@ -62,8 +61,6 @@ export class ReadBinaryFiles implements INodeType { const items: INodeExecutionData[] = []; for (const filePath of files) { - checkFilePathAccess(filePath); - const stream = await this.helpers.createReadStream(filePath); items.push({ binary: { diff --git a/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts b/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts index 29b3529a8532f..71359467e4489 100644 --- a/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts +++ b/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts @@ -6,10 +6,8 @@ import type { } from 'n8n-workflow'; import { BINARY_ENCODING } from 'n8n-workflow'; -import { writeFile as fsWriteFile } from 'fs/promises'; import type { Readable } from 'stream'; -import { checkFilePathAccess } from '@utils/utilities'; import { allowedPathsNotice } from '@utils/descriptions'; export class WriteBinaryFile implements INodeType { @@ -77,8 +75,6 @@ export class WriteBinaryFile implements INodeType { const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex); const fileName = this.getNodeParameter('fileName', itemIndex) as string; - checkFilePathAccess(fileName); - const options = this.getNodeParameter('options', 0, {}); const flag = options.append ? 'a' : 'w'; @@ -103,7 +99,7 @@ export class WriteBinaryFile implements INodeType { } // Write the file to disk - await fsWriteFile(fileName, fileContent, { encoding: 'binary', flag }); + await this.helpers.writeContentToFile(fileName, fileContent, flag); if (item.binary !== undefined) { // Create a shallow copy of the binary data so that the old diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index e9d4ed1f61091..7d38f2b496746 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -7,7 +7,6 @@ import type { Readable } from 'stream'; import type { URLSearchParams } from 'url'; import type { OptionsWithUri, OptionsWithUrl } from 'request'; import type { RequestPromiseOptions, RequestPromiseAPI } from 'request-promise-native'; -import type { PathLike } from 'fs'; import type { CODE_EXECUTION_MODES, CODE_LANGUAGES } from './Constants'; import type { IDeferredPromise } from './DeferredPromise'; @@ -670,7 +669,8 @@ interface JsonHelperFunctions { } export interface FileSystemHelperFunctions { - createReadStream(path: PathLike): Promise; + createReadStream(path: string): Promise; + writeContentToFile(path: string, content: Buffer | Readable, flag: 'a' | 'w'): Promise; } export interface BinaryHelperFunctions {