diff --git a/src/main/services/BackupManager.ts b/src/main/services/BackupManager.ts index 5af4c9f674..46b78ed5a9 100644 --- a/src/main/services/BackupManager.ts +++ b/src/main/services/BackupManager.ts @@ -797,10 +797,11 @@ class BackupManager { async deleteTempBackup(_: Electron.IpcMainInvokeEvent, filePath: string): Promise { try { // Security check: only allow deletion within temp directory - const tempBase = path.join(app.getPath('temp'), 'cherry-studio', 'lan-transfer') - const resolvedPath = path.resolve(filePath) + const tempBase = path.normalize(path.join(app.getPath('temp'), 'cherry-studio', 'lan-transfer')) + const resolvedPath = path.normalize(path.resolve(filePath)) - if (!resolvedPath.startsWith(tempBase)) { + // Use normalized paths with trailing separator to prevent prefix attacks (e.g., /temp-evil) + if (!resolvedPath.startsWith(tempBase + path.sep) && resolvedPath !== tempBase) { logger.warn(`[BackupManager] Attempted to delete file outside temp directory: ${filePath}`) return false } diff --git a/src/main/services/lanTransfer/LanTransferClientService.ts b/src/main/services/lanTransfer/LanTransferClientService.ts index fb80192486..a9335e8f43 100644 --- a/src/main/services/lanTransfer/LanTransferClientService.ts +++ b/src/main/services/lanTransfer/LanTransferClientService.ts @@ -53,6 +53,9 @@ class LanTransferClientService { private isConnecting = false private activeTransfer?: ActiveFileTransfer private lastConnectOptions?: LocalTransferConnectPayload + private consecutiveJsonErrors = 0 + private static readonly MAX_CONSECUTIVE_JSON_ERRORS = 3 + private reconnectPromise: Promise | null = null constructor() { this.responseManager.setTimeoutCallback(() => void this.disconnect()) @@ -296,8 +299,9 @@ class LanTransferClientService { transferId, reason: 'Cancelled by user' }) - } catch { - // Ignore errors when sending cancel message + } catch (error) { + // Expected when connection is already broken + logger.warn('Failed to send cancel message', error as Error) } abortTransfer(this.activeTransfer, new Error('Transfer cancelled by user')) @@ -317,8 +321,24 @@ class LanTransferClientService { throw new Error('No active connection. Please connect to a peer first.') } + // Prevent concurrent reconnection attempts + if (this.reconnectPromise) { + logger.debug('Waiting for existing reconnection attempt...') + await this.reconnectPromise + return + } + logger.info('Connection lost, attempting to reconnect...') - await this.connectAndHandshake(this.lastConnectOptions) + this.reconnectPromise = this.connectAndHandshake(this.lastConnectOptions) + .then(() => { + this.reconnectPromise = null + }) + .catch((error) => { + this.reconnectPromise = null + throw error + }) + + await this.reconnectPromise } private async performFileTransfer( @@ -393,8 +413,21 @@ class LanTransferClientService { let payload: Record try { payload = JSON.parse(line) + this.consecutiveJsonErrors = 0 // Reset on successful parse } catch { - logger.warn('Received invalid JSON control message', { line }) + this.consecutiveJsonErrors++ + logger.warn('Received invalid JSON control message', { line, consecutiveErrors: this.consecutiveJsonErrors }) + + if (this.consecutiveJsonErrors >= LanTransferClientService.MAX_CONSECUTIVE_JSON_ERRORS) { + const message = `Protocol error: ${this.consecutiveJsonErrors} consecutive invalid messages` + logger.error(message) + this.broadcastClientEvent({ + type: 'error', + message, + timestamp: Date.now() + }) + this.consecutiveJsonErrors = 0 + } return } diff --git a/src/main/services/lanTransfer/binaryProtocol.ts b/src/main/services/lanTransfer/binaryProtocol.ts index a3e16e7768..fc1f57a0b2 100644 --- a/src/main/services/lanTransfer/binaryProtocol.ts +++ b/src/main/services/lanTransfer/binaryProtocol.ts @@ -23,6 +23,10 @@ export const BINARY_TYPE_FILE_CHUNK = 0x01 * @returns true if data was buffered, false if backpressure should be applied */ export function sendBinaryChunk(socket: Socket, transferId: string, chunkIndex: number, data: Buffer): boolean { + if (!socket || socket.destroyed || !socket.writable) { + throw new Error('Socket is not writable') + } + const tidBuffer = Buffer.from(transferId, 'utf8') const tidLen = tidBuffer.length diff --git a/src/main/services/lanTransfer/handlers/connection.ts b/src/main/services/lanTransfer/handlers/connection.ts index c3e244bbb5..5a53eeb373 100644 --- a/src/main/services/lanTransfer/handlers/connection.ts +++ b/src/main/services/lanTransfer/handlers/connection.ts @@ -9,6 +9,9 @@ import type { ConnectionContext } from '../types' export const HANDSHAKE_PROTOCOL_VERSION = '1' +/** Maximum size for line buffer to prevent memory exhaustion from malicious peers */ +const MAX_LINE_BUFFER_SIZE = 1024 * 1024 // 1MB limit for control messages + const logger = loggerService.withContext('LanTransferConnection') /** @@ -74,6 +77,14 @@ export function createDataHandler(onControlLine: (line: string) => void): { }, handleData(chunk: Buffer) { lineBuffer += chunk.toString('utf8') + + // Prevent memory exhaustion from malicious peers sending data without newlines + if (lineBuffer.length > MAX_LINE_BUFFER_SIZE) { + logger.error('Line buffer exceeded maximum size, resetting') + lineBuffer = '' + throw new Error('Control message too large') + } + let newlineIndex = lineBuffer.indexOf('\n') while (newlineIndex !== -1) { const line = lineBuffer.slice(0, newlineIndex).trim() diff --git a/src/main/services/lanTransfer/handlers/fileTransfer.ts b/src/main/services/lanTransfer/handlers/fileTransfer.ts index 733de9ec66..86ea61e132 100644 --- a/src/main/services/lanTransfer/handlers/fileTransfer.ts +++ b/src/main/services/lanTransfer/handlers/fileTransfer.ts @@ -32,8 +32,17 @@ export async function validateFile(filePath: string): Promise<{ stats: fs.Stats; let stats: fs.Stats try { stats = await fs.promises.stat(filePath) - } catch { - throw new Error(`File not found: ${filePath}`) + } catch (error) { + const nodeError = error as NodeJS.ErrnoException + if (nodeError.code === 'ENOENT') { + throw new Error(`File not found: ${filePath}`) + } else if (nodeError.code === 'EACCES') { + throw new Error(`Permission denied: ${filePath}`) + } else if (nodeError.code === 'ENOTDIR') { + throw new Error(`Invalid path: ${filePath}`) + } else { + throw new Error(`Cannot access file: ${filePath} (${nodeError.code || 'unknown error'})`) + } } if (!stats.isFile()) {