From f6bf9e9887bb46bb884fd06f3e789f8585445501 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 31 Jul 2023 15:20:39 +0300 Subject: [PATCH] fix(core): Restrict read/write file paths access (#6582) --- packages/cli/src/config/schema.ts | 12 +++ packages/core/src/Constants.ts | 7 ++ packages/core/src/NodeExecuteFunctions.ts | 97 ++++++++++++++++++- .../ReadBinaryFile/ReadBinaryFile.node.ts | 4 +- .../ReadBinaryFiles/ReadBinaryFiles.node.ts | 1 + .../WriteBinaryFile/WriteBinaryFile.node.ts | 15 +-- packages/nodes-base/utils/utilities.ts | 18 ++-- packages/workflow/src/Interfaces.ts | 5 + 8 files changed, 141 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 31fdeb943c0e4..7f93974a86979 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -482,6 +482,18 @@ export const schema = { }, security: { + restrictFileAccessTo: { + doc: 'If set only files in that directories can be accessed. Multiple directories can be separated by semicolon (";").', + format: String, + default: '', + env: 'N8N_RESTRICT_FILE_ACCESS_TO', + }, + blockFileAccessToN8nFiles: { + doc: 'If set to true it will block access to all files in the ".n8n" directory and user defined config files.', + format: Boolean, + default: true, + env: 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES', + }, audit: { daysAbandonedWorkflow: { doc: 'Days for a workflow to be considered abandoned if not executed', diff --git a/packages/core/src/Constants.ts b/packages/core/src/Constants.ts index 84cb1f5ba2c80..f3fc5149e3360 100644 --- a/packages/core/src/Constants.ts +++ b/packages/core/src/Constants.ts @@ -14,3 +14,10 @@ export const RESPONSE_ERROR_MESSAGES = { }; export const CUSTOM_NODES_CATEGORY = 'Custom Nodes'; + +export const RESTRICT_FILE_ACCESS_TO = 'N8N_RESTRICT_FILE_ACCESS_TO'; +export const BLOCK_FILE_ACCESS_TO_N8N_FILES = 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES'; +export const CONFIG_FILES = 'N8N_CONFIG_FILES'; +export const BINARY_DATA_STORAGE_PATH = 'N8N_BINARY_DATA_STORAGE_PATH'; +export const UM_EMAIL_TEMPLATES_INVITE = 'N8N_UM_EMAIL_TEMPLATES_INVITE'; +export const UM_EMAIL_TEMPLATES_PWRESET = 'N8N_UM_EMAIL_TEMPLATES_PWRESET'; diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index f4ea6a3fab39a..3703785079c2e 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -112,14 +112,23 @@ 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'; import type { ExtendedValidationResult, IResponseError, IWorkflowSettings } from './Interfaces'; import { extractValue } from './ExtractValue'; import { getClientCredentialsToken } from './OAuth2Helper'; -import { PLACEHOLDER_EMPTY_EXECUTION_ID } from './Constants'; +import { + CUSTOM_EXTENSION_ENV, + PLACEHOLDER_EMPTY_EXECUTION_ID, + BLOCK_FILE_ACCESS_TO_N8N_FILES, + RESTRICT_FILE_ACCESS_TO, + CONFIG_FILES, + BINARY_DATA_STORAGE_PATH, + UM_EMAIL_TEMPLATES_INVITE, + UM_EMAIL_TEMPLATES_PWRESET, +} from './Constants'; import { binaryToBuffer } from './BinaryDataManager/utils'; import { getAllWorkflowExecutionMetadata, @@ -2240,6 +2249,72 @@ const getRequestHelperFunctions = ( }, }); +const getAllowedPaths = () => { + const restrictFileAccessTo = process.env[RESTRICT_FILE_ACCESS_TO]; + if (!restrictFileAccessTo) { + return []; + } + const allowedPaths = restrictFileAccessTo + .split(';') + .map((path) => path.trim()) + .filter((path) => path); + return allowedPaths; +}; + +function isFilePathBlocked(filePath: string): boolean { + const allowedPaths = getAllowedPaths(); + const resolvedFilePath = path.resolve(filePath); + const userFolder = getUserN8nFolderPath(); + const blockFileAccessToN8nFiles = process.env[BLOCK_FILE_ACCESS_TO_N8N_FILES] !== 'false'; + + //if allowed paths are defined, allow access only to those paths + if (allowedPaths.length) { + for (const path of allowedPaths) { + if (resolvedFilePath.startsWith(path)) { + return false; + } + } + + return true; + } + + //restrict access to .n8n folder and other .env config related paths + if (blockFileAccessToN8nFiles) { + const restrictedPaths: string[] = [userFolder]; + + if (process.env[CONFIG_FILES]) { + restrictedPaths.push(...process.env[CONFIG_FILES].split(',')); + } + + if (process.env[CUSTOM_EXTENSION_ENV]) { + const customExtensionFolders = process.env[CUSTOM_EXTENSION_ENV].split(';'); + restrictedPaths.push(...customExtensionFolders); + } + + if (process.env[BINARY_DATA_STORAGE_PATH]) { + restrictedPaths.push(process.env[BINARY_DATA_STORAGE_PATH]); + } + + if (process.env[UM_EMAIL_TEMPLATES_INVITE]) { + restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_INVITE]); + } + + if (process.env[UM_EMAIL_TEMPLATES_PWRESET]) { + restrictedPaths.push(process.env[UM_EMAIL_TEMPLATES_PWRESET]); + } + + //check if the file path is restricted + for (const path of restrictedPaths) { + if (resolvedFilePath.startsWith(path)) { + return true; + } + } + } + + //path is not restricted + return false; +} + const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => ({ async createReadStream(filePath) { try { @@ -2248,14 +2323,32 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => throw error.code === 'ENOENT' ? new NodeOperationError(node, error, { message: `The file "${String(filePath)}" could not be accessed.`, + severity: 'warning', }) : error; } + if (isFilePathBlocked(filePath as string)) { + const allowedPaths = getAllowedPaths(); + const message = allowedPaths.length ? ` Allowed paths: ${allowedPaths.join(', ')}` : ''; + throw new NodeOperationError(node, `Access to the file is not allowed.${message}`, { + severity: 'warning', + }); + } return createReadStream(filePath); }, + getStoragePath() { return path.join(getUserN8nFolderPath(), `storage/${node.type}`); }, + + async writeContentToFile(filePath, content, flag) { + if (isFilePathBlocked(filePath as string)) { + 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 00b591ee61945..4287b369eb71f 100644 --- a/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts +++ b/packages/nodes-base/nodes/ReadBinaryFile/ReadBinaryFile.node.ts @@ -67,15 +67,17 @@ export class ReadBinaryFile implements INodeType { } const filePath = this.getNodeParameter('filePath', itemIndex); + const stream = await this.helpers.createReadStream(filePath); const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion newItem.binary![dataPropertyName] = await this.helpers.prepareBinaryData(stream, filePath); returnData.push(newItem); } catch (error) { if (this.continueOnFail()) { returnData.push({ json: { - error: error.message, + error: (error as Error).message, }, pairedItem: { item: itemIndex, diff --git a/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts b/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts index bd9ebcb666d88..78887d0f16d6f 100644 --- a/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts +++ b/packages/nodes-base/nodes/ReadBinaryFiles/ReadBinaryFiles.node.ts @@ -4,6 +4,7 @@ import type { INodeType, INodeTypeDescription, } from 'n8n-workflow'; + import glob from 'fast-glob'; export class ReadBinaryFiles implements INodeType { diff --git a/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts b/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts index 176f7fa4e2bb6..ed08ccec36077 100644 --- a/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts +++ b/packages/nodes-base/nodes/WriteBinaryFile/WriteBinaryFile.node.ts @@ -1,13 +1,12 @@ +import type { Readable } from 'stream'; + +import { BINARY_ENCODING } from 'n8n-workflow'; import type { IExecuteFunctions, INodeExecutionData, INodeType, INodeTypeDescription, } from 'n8n-workflow'; -import { BINARY_ENCODING } from 'n8n-workflow'; - -import { writeFile as fsWriteFile } from 'fs/promises'; -import type { Readable } from 'stream'; export class WriteBinaryFile implements INodeType { description: INodeTypeDescription = { @@ -73,9 +72,10 @@ export class WriteBinaryFile implements INodeType { const dataPropertyName = this.getNodeParameter('dataPropertyName', itemIndex); const fileName = this.getNodeParameter('fileName', itemIndex) as string; + const options = this.getNodeParameter('options', 0, {}); - const flag = options.append ? 'a' : 'w'; + const flag: string = options.append ? 'a' : 'w'; item = items[itemIndex]; @@ -97,7 +97,8 @@ export class WriteBinaryFile implements INodeType { } // Write the file to disk - await fsWriteFile(fileName, fileContent, { encoding: 'binary', flag }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + await this.helpers.writeContentToFile(fileName, fileContent, flag); if (item.binary !== undefined) { // Create a shallow copy of the binary data so that the old @@ -116,7 +117,7 @@ export class WriteBinaryFile implements INodeType { if (this.continueOnFail()) { returnData.push({ json: { - error: error.message, + error: (error as Error).message, }, pairedItem: { item: itemIndex, diff --git a/packages/nodes-base/utils/utilities.ts b/packages/nodes-base/utils/utilities.ts index 4b7ce379899ee..6de1b90978889 100644 --- a/packages/nodes-base/utils/utilities.ts +++ b/packages/nodes-base/utils/utilities.ts @@ -25,8 +25,8 @@ import { isEqual, isNull, merge } from 'lodash'; * // => [['a', 'b', 'c'], ['d']] */ -export function chunk(array: any[], size = 1) { - const length = array == null ? 0 : array.length; +export function chunk(array: T[], size = 1) { + const length = array === null ? 0 : array.length; if (!length || size < 1) { return []; } @@ -37,7 +37,7 @@ export function chunk(array: any[], size = 1) { while (index < length) { result[resIndex++] = array.slice(index, (index += size)); } - return result; + return result as T[][]; } /** @@ -51,20 +51,22 @@ export function chunk(array: any[], size = 1) { * */ -export function flatten(nestedArray: any[][]) { +export function flatten(nestedArray: T[][]) { const result = []; - (function loop(array: any[] | any) { + (function loop(array: T[] | T[][]) { for (let i = 0; i < array.length; i++) { if (Array.isArray(array[i])) { - loop(array[i]); + loop(array[i] as T[]); } else { result.push(array[i]); } } })(nestedArray); - return result; + //TODO: check logic in MicrosoftSql.node.ts + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-return + return result as any; } export function updateDisplayOptions( @@ -210,7 +212,7 @@ export function wrapData(data: IDataObject | IDataObject[]): INodeExecutionData[ export const keysToLowercase = (headers: T) => { if (typeof headers !== 'object' || Array.isArray(headers) || headers === null) return headers; return Object.entries(headers).reduce((acc, [key, value]) => { - acc[key.toLowerCase()] = value; + acc[key.toLowerCase()] = value as IDataObject; return acc; }, {} as IDataObject); }; diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 9da2110411e52..3b08473fca0d8 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -672,6 +672,11 @@ interface JsonHelperFunctions { export interface FileSystemHelperFunctions { createReadStream(path: PathLike): Promise; getStoragePath(): string; + writeContentToFile( + path: PathLike, + content: string | Buffer | Readable, + flag?: string, + ): Promise; } export interface BinaryHelperFunctions {