Skip to content

Commit 33cebd7

Browse files
committed
fix(ssh/sftp): cap remote file reads on received bytes, not stat() size
The SFTP/SSH download routes buffered a remote file into memory with the only size guard being sftp.stat().size — a value the caller-supplied SSH server controls. A server that reports a tiny size and then streams endlessly drove unbounded heap growth until OOM. Add readSftpFileCapped(), which counts received bytes and destroys the stream as soon as the cap is exceeded, and route all four SFTP-reading tool routes through it (download, download-file, read-file-content, and the append path of write-file-content, which had no cap at all). Cap breaches now return 400 instead of 500, and responses report the actual byte count rather than the server-reported stat size.
1 parent 3740c62 commit 33cebd7

6 files changed

Lines changed: 201 additions & 77 deletions

File tree

apps/sim/app/api/tools/sftp/download/route.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,15 @@ import { checkInternalAuth } from '@/lib/auth/hybrid'
88
import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1010
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
11-
import { createSftpConnection, getSftp, isPathSafe, sanitizePath } from '@/app/api/tools/sftp/utils'
11+
import {
12+
createSftpConnection,
13+
getSftp,
14+
isPathSafe,
15+
MAX_SFTP_READ_BYTES,
16+
readSftpFileCapped,
17+
SftpFileTooLargeError,
18+
sanitizePath,
19+
} from '@/app/api/tools/sftp/utils'
1220

1321
export const dynamic = 'force-dynamic'
1422

@@ -73,8 +81,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7381
})
7482
})
7583

76-
const maxSize = 50 * 1024 * 1024
77-
if (stats.size > maxSize) {
84+
if (stats.size > MAX_SFTP_READ_BYTES) {
7885
const sizeMB = (stats.size / (1024 * 1024)).toFixed(2)
7986
return NextResponse.json(
8087
{ success: false, error: `File size (${sizeMB}MB) exceeds download limit of 50MB` },
@@ -84,19 +91,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8491

8592
logger.info(`[${requestId}] Downloading file ${remotePath} (${stats.size} bytes)`)
8693

87-
const chunks: Buffer[] = []
88-
await new Promise<void>((resolve, reject) => {
89-
const readStream = sftp.createReadStream(remotePath)
90-
91-
readStream.on('data', (chunk: Buffer) => {
92-
chunks.push(chunk)
93-
})
94-
95-
readStream.on('end', () => resolve())
96-
readStream.on('error', reject)
97-
})
98-
99-
const buffer = Buffer.concat(chunks)
94+
const buffer = await readSftpFileCapped(
95+
sftp,
96+
remotePath,
97+
MAX_SFTP_READ_BYTES,
98+
'File exceeds download limit of 50MB'
99+
)
100100
const fileName = path.basename(remotePath)
101101
const extension = getFileExtension(fileName)
102102
const mimeType = getMimeTypeFromExtension(extension)
@@ -129,6 +129,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
129129
}
130130
} catch (error) {
131131
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
132+
133+
if (error instanceof SftpFileTooLargeError) {
134+
logger.warn(`[${requestId}] SFTP download aborted: ${errorMessage}`)
135+
return NextResponse.json({ success: false, error: errorMessage }, { status: 400 })
136+
}
137+
132138
logger.error(`[${requestId}] SFTP download failed:`, error)
133139

134140
return NextResponse.json({ error: `SFTP download failed: ${errorMessage}` }, { status: 500 })
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { EventEmitter } from 'events'
5+
import type { SFTPWrapper } from 'ssh2'
6+
import { describe, expect, it } from 'vitest'
7+
import {
8+
MAX_SFTP_READ_BYTES,
9+
readSftpFileCapped,
10+
SftpFileTooLargeError,
11+
} from '@/app/api/tools/sftp/utils'
12+
13+
class FakeReadStream extends EventEmitter {
14+
destroyed = false
15+
16+
destroy() {
17+
this.destroyed = true
18+
}
19+
}
20+
21+
/**
22+
* Builds a fake SFTP wrapper whose read stream emits `chunkCount` chunks of
23+
* `chunkSize` bytes — the shape of a malicious server that understates the
24+
* file size in its stat reply and then streams unbounded data.
25+
*/
26+
function fakeSftp(chunkSize: number, chunkCount: number) {
27+
const stream = new FakeReadStream()
28+
const sftp = {
29+
createReadStream: () => {
30+
queueMicrotask(() => {
31+
for (let i = 0; i < chunkCount; i++) {
32+
if (stream.destroyed) return
33+
stream.emit('data', Buffer.alloc(chunkSize, 0x41))
34+
}
35+
if (!stream.destroyed) stream.emit('end')
36+
})
37+
return stream
38+
},
39+
} as unknown as SFTPWrapper
40+
return { sftp, stream }
41+
}
42+
43+
describe('readSftpFileCapped', () => {
44+
it('resolves with the full contents when under the cap', async () => {
45+
const { sftp } = fakeSftp(4, 3)
46+
47+
const buffer = await readSftpFileCapped(sftp, '/file', 1024)
48+
49+
expect(buffer.length).toBe(12)
50+
expect(buffer.toString()).toBe('A'.repeat(12))
51+
})
52+
53+
it('rejects and destroys the stream once received bytes exceed the cap', async () => {
54+
const { sftp, stream } = fakeSftp(8, 100)
55+
56+
await expect(readSftpFileCapped(sftp, '/bomb', 16)).rejects.toBeInstanceOf(
57+
SftpFileTooLargeError
58+
)
59+
expect(stream.destroyed).toBe(true)
60+
})
61+
62+
it('enforces the cap on actual bytes even when the file was reported as tiny', async () => {
63+
const { sftp, stream } = fakeSftp(1024, 1024)
64+
65+
await expect(readSftpFileCapped(sftp, '/bomb', 4096, 'too big')).rejects.toThrow('too big')
66+
expect(stream.destroyed).toBe(true)
67+
})
68+
69+
it('caps remote reads at 50MB', () => {
70+
expect(MAX_SFTP_READ_BYTES).toBe(50 * 1024 * 1024)
71+
})
72+
})

apps/sim/app/api/tools/sftp/utils.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,53 @@ export function getSftp(client: Client): Promise<SFTPWrapper> {
172172
})
173173
}
174174

175+
/** Maximum bytes a route will buffer from a remote SFTP file. */
176+
export const MAX_SFTP_READ_BYTES = 50 * 1024 * 1024
177+
178+
/**
179+
* Thrown when a remote file streams more bytes than the caller's cap allows.
180+
*/
181+
export class SftpFileTooLargeError extends Error {
182+
constructor(message: string) {
183+
super(message)
184+
this.name = 'SftpFileTooLargeError'
185+
}
186+
}
187+
188+
/**
189+
* Reads a remote file into memory, enforcing the cap on the bytes actually
190+
* received rather than on the `stat()` size the remote server reports.
191+
* A caller-supplied SSH server can understate the size in its `SSH_FXP_STAT`
192+
* reply and then stream unbounded data, so the stream is destroyed as soon as
193+
* the running total exceeds `maxBytes`.
194+
*/
195+
export function readSftpFileCapped(
196+
sftp: SFTPWrapper,
197+
remotePath: string,
198+
maxBytes: number,
199+
errorMessage = `File exceeds maximum allowed size of ${maxBytes} bytes`
200+
): Promise<Buffer> {
201+
return new Promise((resolve, reject) => {
202+
const chunks: Buffer[] = []
203+
let totalBytes = 0
204+
const readStream = sftp.createReadStream(remotePath)
205+
206+
readStream.on('data', (chunk: Buffer) => {
207+
totalBytes += chunk.length
208+
if (totalBytes > maxBytes) {
209+
readStream.destroy()
210+
chunks.length = 0
211+
reject(new SftpFileTooLargeError(errorMessage))
212+
return
213+
}
214+
chunks.push(chunk)
215+
})
216+
217+
readStream.on('end', () => resolve(Buffer.concat(chunks)))
218+
readStream.on('error', reject)
219+
})
220+
}
221+
175222
/**
176223
* Sanitizes a remote path to prevent path traversal attacks.
177224
* Removes null bytes, normalizes path separators, and collapses traversal sequences.

apps/sim/app/api/tools/ssh/download-file/route.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import { parseRequest } from '@/lib/api/server'
99
import { checkInternalAuth } from '@/lib/auth/hybrid'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
12+
import {
13+
MAX_SFTP_READ_BYTES,
14+
readSftpFileCapped,
15+
SftpFileTooLargeError,
16+
} from '@/app/api/tools/sftp/utils'
1217
import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils'
1318

1419
const logger = createLogger('SSHDownloadFileAPI')
@@ -67,31 +72,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6772
})
6873
})
6974

70-
// Check file size limit (50MB to prevent memory exhaustion)
71-
const maxSize = 50 * 1024 * 1024
72-
if (stats.size > maxSize) {
75+
if (stats.size > MAX_SFTP_READ_BYTES) {
7376
const sizeMB = (stats.size / (1024 * 1024)).toFixed(2)
7477
return NextResponse.json(
7578
{ error: `File size (${sizeMB}MB) exceeds download limit of 50MB` },
7679
{ status: 400 }
7780
)
7881
}
7982

80-
// Read file content
81-
const content = await new Promise<Buffer>((resolve, reject) => {
82-
const chunks: Buffer[] = []
83-
const readStream = sftp.createReadStream(remotePath)
84-
85-
readStream.on('data', (chunk: Buffer) => {
86-
chunks.push(chunk)
87-
})
88-
89-
readStream.on('end', () => {
90-
resolve(Buffer.concat(chunks))
91-
})
92-
93-
readStream.on('error', reject)
94-
})
83+
const content = await readSftpFileCapped(
84+
sftp,
85+
remotePath,
86+
MAX_SFTP_READ_BYTES,
87+
'File exceeds download limit of 50MB'
88+
)
9589

9690
const fileName = path.basename(remotePath)
9791
const extension = getFileExtension(fileName)
@@ -108,19 +102,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
108102
name: fileName,
109103
mimeType,
110104
data: base64Content,
111-
size: stats.size,
105+
size: content.length,
112106
},
113107
content: base64Content,
114108
fileName: fileName,
115109
remotePath: remotePath,
116-
size: stats.size,
110+
size: content.length,
117111
message: `File downloaded successfully from ${remotePath}`,
118112
})
119113
} finally {
120114
client.end()
121115
}
122116
} catch (error) {
123117
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
118+
119+
if (error instanceof SftpFileTooLargeError) {
120+
logger.warn(`[${requestId}] SSH file download aborted: ${errorMessage}`)
121+
return NextResponse.json({ error: errorMessage }, { status: 400 })
122+
}
123+
124124
logger.error(`[${requestId}] SSH file download failed:`, error)
125125

126126
return NextResponse.json(

apps/sim/app/api/tools/ssh/read-file-content/route.ts

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { sshReadFileContentContract } from '@/lib/api/contracts/storage-transfer
77
import { parseRequest } from '@/lib/api/server'
88
import { checkInternalAuth } from '@/lib/auth/hybrid'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
10+
import { readSftpFileCapped, SftpFileTooLargeError } from '@/app/api/tools/sftp/utils'
1011
import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils'
1112

1213
const logger = createLogger('SSHReadFileContentAPI')
@@ -72,47 +73,38 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7273
)
7374
}
7475

75-
const content = await new Promise<string>((resolve, reject) => {
76-
const chunks: Buffer[] = []
77-
let totalBytes = 0
78-
const readStream = sftp.createReadStream(filePath)
79-
80-
readStream.on('data', (chunk: Buffer) => {
81-
totalBytes += chunk.length
82-
if (totalBytes > maxBytes) {
83-
readStream.destroy()
84-
reject(new Error(`File exceeds maximum allowed size of ${params.maxSize}MB`))
85-
return
86-
}
87-
chunks.push(chunk)
88-
})
89-
90-
readStream.on('end', () => {
91-
const buffer = Buffer.concat(chunks)
92-
resolve(buffer.toString(params.encoding as BufferEncoding))
93-
})
94-
95-
readStream.on('error', reject)
96-
})
76+
const buffer = await readSftpFileCapped(
77+
sftp,
78+
filePath,
79+
maxBytes,
80+
`File exceeds maximum allowed size of ${params.maxSize}MB`
81+
)
82+
const content = buffer.toString(params.encoding as BufferEncoding)
9783

9884
const lines = content.split('\n').length
9985

10086
logger.info(
101-
`[${requestId}] File content read successfully: ${stats.size} bytes, ${lines} lines`
87+
`[${requestId}] File content read successfully: ${buffer.length} bytes, ${lines} lines`
10288
)
10389

10490
return NextResponse.json({
10591
content,
106-
size: stats.size,
92+
size: buffer.length,
10793
lines,
10894
path: filePath,
109-
message: `File read successfully: ${stats.size} bytes, ${lines} lines`,
95+
message: `File read successfully: ${buffer.length} bytes, ${lines} lines`,
11096
})
11197
} finally {
11298
client.end()
11399
}
114100
} catch (error) {
115101
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
102+
103+
if (error instanceof SftpFileTooLargeError) {
104+
logger.warn(`[${requestId}] SSH read file content aborted: ${errorMessage}`)
105+
return NextResponse.json({ error: errorMessage }, { status: 400 })
106+
}
107+
116108
logger.error(`[${requestId}] SSH read file content failed:`, error)
117109

118110
return NextResponse.json(

0 commit comments

Comments
 (0)