From c2b830f279e454e8b758da441016b2234f220ac7 Mon Sep 17 00:00:00 2001 From: Henry Heng Date: Thu, 13 Mar 2025 19:46:27 +0000 Subject: [PATCH] Bugfix/arbitrary create attachemnt file upload (#4171) fix arbitrary create attachemnt file upload --- .../PostgresRecordManager.ts | 1 - packages/components/src/index.ts | 1 + packages/components/src/storageUtils.ts | 12 +++---- packages/components/src/validator.ts | 29 +++++++++++++++++ packages/server/src/utils/createAttachment.ts | 32 +++++++++++++------ 5 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 packages/components/src/validator.ts diff --git a/packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts b/packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts index 0c5d60fb8..6294429df 100644 --- a/packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts +++ b/packages/components/nodes/recordmanager/PostgresRecordManager/PostgresRecordManager.ts @@ -4,7 +4,6 @@ import { ListKeyOptions, RecordManagerInterface, UpdateOptions } from '@langchai import { DataSource } from 'typeorm' import { getHost, getSSL } from '../../vectorstores/Postgres/utils' import { getDatabase, getPort, getTableName } from './utils' -import fs from 'fs' const serverCredentialsExists = !!process.env.POSTGRES_RECORDMANAGER_USER && !!process.env.POSTGRES_RECORDMANAGER_PASSWORD diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index ec713bec3..a38761066 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -10,3 +10,4 @@ export * from './speechToText' export * from './storageUtils' export * from './handler' export * from './followUpPrompts' +export * from './validator' diff --git a/packages/components/src/storageUtils.ts b/packages/components/src/storageUtils.ts index 3f120b700..7d2142208 100644 --- a/packages/components/src/storageUtils.ts +++ b/packages/components/src/storageUtils.ts @@ -77,7 +77,7 @@ export const addArrayFilesToStorage = async (mime: string, bf: Buffer, fileName: fileNames.push(sanitizedFilename) return 'FILE-STORAGE::' + JSON.stringify(fileNames) } else { - const dir = path.join(getStoragePath(), ...paths) + const dir = path.join(getStoragePath(), ...paths.map(_sanitizeFilename)) if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }) } @@ -110,7 +110,7 @@ export const addSingleFileToStorage = async (mime: string, bf: Buffer, fileName: await s3Client.send(putObjCmd) return 'FILE-STORAGE::' + sanitizedFilename } else { - const dir = path.join(getStoragePath(), ...paths) + const dir = path.join(getStoragePath(), ...paths.map(_sanitizeFilename)) if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }) } @@ -180,7 +180,7 @@ export const getFileFromStorage = async (file: string, ...paths: string[]): Prom const buffer = Buffer.concat(response.Body.toArray()) return buffer } else { - const fileInStorage = path.join(getStoragePath(), ...paths, sanitizedFilename) + const fileInStorage = path.join(getStoragePath(), ...paths.map(_sanitizeFilename), sanitizedFilename) return fs.readFileSync(fileInStorage) } } @@ -209,7 +209,7 @@ export const removeFilesFromStorage = async (...paths: string[]) => { } await _deleteS3Folder(Key) } else { - const directory = path.join(getStoragePath(), ...paths) + const directory = path.join(getStoragePath(), ...paths.map(_sanitizeFilename)) _deleteLocalFolderRecursive(directory) } } @@ -243,7 +243,7 @@ export const removeSpecificFileFromStorage = async (...paths: string[]) => { const sanitizedFilename = _sanitizeFilename(fileName) paths.push(sanitizedFilename) } - const file = path.join(getStoragePath(), ...paths) + const file = path.join(getStoragePath(), ...paths.map(_sanitizeFilename)) fs.unlinkSync(file) } } @@ -258,7 +258,7 @@ export const removeFolderFromStorage = async (...paths: string[]) => { } await _deleteS3Folder(Key) } else { - const directory = path.join(getStoragePath(), ...paths) + const directory = path.join(getStoragePath(), ...paths.map(_sanitizeFilename)) _deleteLocalFolderRecursive(directory, true) } } diff --git a/packages/components/src/validator.ts b/packages/components/src/validator.ts new file mode 100644 index 000000000..4948165eb --- /dev/null +++ b/packages/components/src/validator.ts @@ -0,0 +1,29 @@ +/** + * Validates if a string is a valid UUID v4 + * @param {string} uuid The string to validate + * @returns {boolean} True if valid UUID, false otherwise + */ +export const isValidUUID = (uuid: string): boolean => { + // UUID v4 regex pattern + const uuidV4Pattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + return uuidV4Pattern.test(uuid) +} + +/** + * Validates if a string contains path traversal attempts + * @param {string} path The string to validate + * @returns {boolean} True if path traversal detected, false otherwise + */ +export const isPathTraversal = (path: string): boolean => { + // Check for common path traversal patterns + const dangerousPatterns = [ + '..', // Directory traversal + '/', // Root directory + '\\', // Windows root directory + '%2e', // URL encoded . + '%2f', // URL encoded / + '%5c' // URL encoded \ + ] + + return dangerousPatterns.some((pattern) => path.toLowerCase().includes(pattern)) +} diff --git a/packages/server/src/utils/createAttachment.ts b/packages/server/src/utils/createAttachment.ts index 3a2e691a8..d38f6a93c 100644 --- a/packages/server/src/utils/createAttachment.ts +++ b/packages/server/src/utils/createAttachment.ts @@ -6,10 +6,15 @@ import { IDocument, mapExtToInputField, mapMimeTypeToInputField, - removeSpecificFileFromUpload + removeSpecificFileFromUpload, + isValidUUID, + isPathTraversal } from 'flowise-components' import { getRunningExpressApp } from './getRunningExpressApp' import { getErrorMessage } from '../errors/utils' +import { InternalFlowiseError } from '../errors/internalFlowiseError' +import { StatusCodes } from 'http-status-codes' +import { ChatFlow } from '../database/entities/ChatFlow' /** * Create attachment @@ -19,17 +24,26 @@ export const createFileAttachment = async (req: Request) => { const appServer = getRunningExpressApp() const chatflowid = req.params.chatflowId - if (!chatflowid) { - throw new Error( - 'Params chatflowId is required! Please provide chatflowId and chatId in the URL: /api/v1/attachments/:chatflowId/:chatId' - ) + if (!chatflowid || !isValidUUID(chatflowid)) { + throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, 'Invalid chatflowId format - must be a valid UUID') } const chatId = req.params.chatId - if (!chatId) { - throw new Error( - 'Params chatId is required! Please provide chatflowId and chatId in the URL: /api/v1/attachments/:chatflowId/:chatId' - ) + if (!chatId || !isValidUUID(chatId)) { + throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, 'Invalid chatId format - must be a valid UUID') + } + + // Check for path traversal attempts + if (isPathTraversal(chatflowid) || isPathTraversal(chatId)) { + throw new InternalFlowiseError(StatusCodes.BAD_REQUEST, 'Invalid path characters detected') + } + + // Validate chatflow exists and check API key + const chatflow = await appServer.AppDataSource.getRepository(ChatFlow).findOneBy({ + id: chatflowid + }) + if (!chatflow) { + throw new InternalFlowiseError(StatusCodes.NOT_FOUND, `Chatflow ${chatflowid} not found`) } // Find FileLoader node