Skip to content

Commit fb510d1

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. Read through the existing readNodeStreamToBufferWithLimit limiter, which enforces the cap on bytes actually received and destroys the stream on breach, via a small readSftpFileCapped wrapper in sftp/utils. All four SFTP-reading tool routes now share it — download, download-file, read-file-content, and the append path of write-file-content, which had no cap at all. The wrapper keeps a no-op error listener on the stream for its whole life: ssh2 rejects still-pending SFTP requests when the channel closes, which arrives as a late error event after the limiter has detached its own handlers, and an error event with no listener would take the process down. Too-large responses now return 413 to match how the rest of the routes surface PayloadSizeLimitError, and responses report the actual byte count rather than the server-reported stat size.
1 parent 3740c62 commit fb510d1

6 files changed

Lines changed: 178 additions & 80 deletions

File tree

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

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,17 @@ import { sftpDownloadContract } from '@/lib/api/contracts/storage-transfer'
66
import { parseRequest } from '@/lib/api/server'
77
import { checkInternalAuth } from '@/lib/auth/hybrid'
88
import { generateRequestId } from '@/lib/core/utils/request'
9+
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1011
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
11-
import { createSftpConnection, getSftp, isPathSafe, sanitizePath } from '@/app/api/tools/sftp/utils'
12+
import {
13+
createSftpConnection,
14+
getSftp,
15+
isPathSafe,
16+
MAX_SFTP_READ_BYTES,
17+
readSftpFileCapped,
18+
sanitizePath,
19+
} from '@/app/api/tools/sftp/utils'
1220

1321
export const dynamic = 'force-dynamic'
1422

@@ -73,30 +81,22 @@ 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` },
81-
{ status: 400 }
88+
{ status: 413 }
8289
)
8390
}
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+
'SFTP download'
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 (isPayloadSizeLimitError(error)) {
134+
logger.warn(`[${requestId}] SFTP download aborted: ${errorMessage}`)
135+
return NextResponse.json({ success: false, error: errorMessage }, { status: 413 })
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 { Readable } from 'stream'
5+
import type { SFTPWrapper } from 'ssh2'
6+
import { describe, expect, it, vi } from 'vitest'
7+
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
8+
import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils'
9+
10+
/**
11+
* Builds a fake SFTP wrapper whose read stream emits `chunkCount` chunks of
12+
* `chunkSize` bytes — the shape of a malicious server that understates the
13+
* file size in its stat reply and then streams unbounded data.
14+
*/
15+
function fakeSftp(chunkSize: number, chunkCount: number) {
16+
let emitted = 0
17+
const stream = new Readable({
18+
read() {
19+
if (emitted >= chunkCount) {
20+
this.push(null)
21+
return
22+
}
23+
emitted++
24+
this.push(Buffer.alloc(chunkSize, 0x41))
25+
},
26+
})
27+
const createReadStream = vi.fn(() => stream)
28+
return { sftp: { createReadStream } as unknown as SFTPWrapper, stream, createReadStream }
29+
}
30+
31+
describe('readSftpFileCapped', () => {
32+
it('resolves with the full contents when under the cap', async () => {
33+
const { sftp, createReadStream } = fakeSftp(4, 3)
34+
35+
const buffer = await readSftpFileCapped(sftp, '/file', 1024, 'file')
36+
37+
expect(buffer.toString()).toBe('A'.repeat(12))
38+
expect(createReadStream).toHaveBeenCalledWith('/file')
39+
})
40+
41+
it('rejects and destroys the stream once received bytes exceed the cap', async () => {
42+
const { sftp, stream } = fakeSftp(8, 1_000_000)
43+
44+
await expect(readSftpFileCapped(sftp, '/bomb', 16, 'file')).rejects.toSatisfy(
45+
isPayloadSizeLimitError
46+
)
47+
expect(stream.destroyed).toBe(true)
48+
})
49+
50+
it('enforces the cap on actual bytes even when the file was reported as tiny', async () => {
51+
const { sftp, stream } = fakeSftp(1024, 1_000_000)
52+
53+
await expect(readSftpFileCapped(sftp, '/bomb', 4096, 'file')).rejects.toThrow(
54+
/exceeds maximum size of 4096 bytes/
55+
)
56+
expect(stream.destroyed).toBe(true)
57+
})
58+
59+
it('survives the late stream error ssh2 emits when the channel closes after an abort', async () => {
60+
const { sftp, stream } = fakeSftp(8, 1_000_000)
61+
62+
await expect(readSftpFileCapped(sftp, '/bomb', 16, 'file')).rejects.toSatisfy(
63+
isPayloadSizeLimitError
64+
)
65+
66+
expect(() => stream.emit('error', new Error('No response from server'))).not.toThrow()
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: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { toError } from '@sim/utils/errors'
22
import { type Attributes, Client, type ConnectConfig, type SFTPWrapper } from 'ssh2'
33
import { validateDatabaseHost } from '@/lib/core/security/input-validation.server'
4+
import { readNodeStreamToBufferWithLimit } from '@/lib/core/utils/stream-limits'
45

56
const S_IFMT = 0o170000
67
const S_IFDIR = 0o040000
@@ -172,6 +173,36 @@ export function getSftp(client: Client): Promise<SFTPWrapper> {
172173
})
173174
}
174175

176+
/** Maximum bytes a route will buffer from a remote SFTP file. */
177+
export const MAX_SFTP_READ_BYTES = 50 * 1024 * 1024
178+
179+
/**
180+
* Reads a remote file into memory, enforcing the cap on the bytes actually
181+
* received rather than on the `stat()` size the remote server reports.
182+
* A caller-supplied SSH server can understate the size in its `SSH_FXP_STAT`
183+
* reply and then stream unbounded data, so the stream is destroyed as soon as
184+
* the running total exceeds `maxBytes`. Rejects with a `PayloadSizeLimitError`.
185+
*/
186+
export function readSftpFileCapped(
187+
sftp: SFTPWrapper,
188+
remotePath: string,
189+
maxBytes: number,
190+
label: string
191+
): Promise<Buffer> {
192+
const stream = sftp.createReadStream(remotePath)
193+
194+
/**
195+
* Closing the SSH client rejects every still-pending SFTP request with
196+
* "No response from server", which lands as a late `error` on a stream the
197+
* limiter has already detached from once it destroyed it. An `error` event
198+
* with no listener is an uncaught exception, so keep one attached for the
199+
* stream's whole life; the limiter's own handler still settles the promise.
200+
*/
201+
stream.on('error', () => {})
202+
203+
return readNodeStreamToBufferWithLimit(stream, { maxBytes, label })
204+
}
205+
175206
/**
176207
* Sanitizes a remote path to prevent path traversal attacks.
177208
* Removes null bytes, normalizes path separators, and collapses traversal sequences.

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

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import type { Client, SFTPWrapper } from 'ssh2'
77
import { sshDownloadFileContract } from '@/lib/api/contracts/storage-transfer'
88
import { parseRequest } from '@/lib/api/server'
99
import { checkInternalAuth } from '@/lib/auth/hybrid'
10+
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1011
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1112
import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
13+
import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils'
1214
import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils'
1315

1416
const logger = createLogger('SSHDownloadFileAPI')
@@ -67,31 +69,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6769
})
6870
})
6971

70-
// Check file size limit (50MB to prevent memory exhaustion)
71-
const maxSize = 50 * 1024 * 1024
72-
if (stats.size > maxSize) {
72+
if (stats.size > MAX_SFTP_READ_BYTES) {
7373
const sizeMB = (stats.size / (1024 * 1024)).toFixed(2)
7474
return NextResponse.json(
7575
{ error: `File size (${sizeMB}MB) exceeds download limit of 50MB` },
76-
{ status: 400 }
76+
{ status: 413 }
7777
)
7878
}
7979

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-
})
80+
const content = await readSftpFileCapped(
81+
sftp,
82+
remotePath,
83+
MAX_SFTP_READ_BYTES,
84+
'SSH file download'
85+
)
9586

9687
const fileName = path.basename(remotePath)
9788
const extension = getFileExtension(fileName)
@@ -108,19 +99,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
10899
name: fileName,
109100
mimeType,
110101
data: base64Content,
111-
size: stats.size,
102+
size: content.length,
112103
},
113104
content: base64Content,
114105
fileName: fileName,
115106
remotePath: remotePath,
116-
size: stats.size,
107+
size: content.length,
117108
message: `File downloaded successfully from ${remotePath}`,
118109
})
119110
} finally {
120111
client.end()
121112
}
122113
} catch (error) {
123114
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
115+
116+
if (isPayloadSizeLimitError(error)) {
117+
logger.warn(`[${requestId}] SSH file download aborted: ${errorMessage}`)
118+
return NextResponse.json({ error: errorMessage }, { status: 413 })
119+
}
120+
124121
logger.error(`[${requestId}] SSH file download failed:`, error)
125122

126123
return NextResponse.json(

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

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import type { Client, SFTPWrapper } from 'ssh2'
66
import { sshReadFileContentContract } from '@/lib/api/contracts/storage-transfer'
77
import { parseRequest } from '@/lib/api/server'
88
import { checkInternalAuth } from '@/lib/auth/hybrid'
9+
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { readSftpFileCapped } from '@/app/api/tools/sftp/utils'
1012
import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils'
1113

1214
const logger = createLogger('SSHReadFileContentAPI')
@@ -68,51 +70,37 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6870
if (stats.size > maxBytes) {
6971
return NextResponse.json(
7072
{ error: `File size (${stats.size} bytes) exceeds maximum allowed (${maxBytes} bytes)` },
71-
{ status: 400 }
73+
{ status: 413 }
7274
)
7375
}
7476

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-
})
77+
const buffer = await readSftpFileCapped(sftp, filePath, maxBytes, `File '${filePath}'`)
78+
const content = buffer.toString(params.encoding as BufferEncoding)
9779

9880
const lines = content.split('\n').length
9981

10082
logger.info(
101-
`[${requestId}] File content read successfully: ${stats.size} bytes, ${lines} lines`
83+
`[${requestId}] File content read successfully: ${buffer.length} bytes, ${lines} lines`
10284
)
10385

10486
return NextResponse.json({
10587
content,
106-
size: stats.size,
88+
size: buffer.length,
10789
lines,
10890
path: filePath,
109-
message: `File read successfully: ${stats.size} bytes, ${lines} lines`,
91+
message: `File read successfully: ${buffer.length} bytes, ${lines} lines`,
11092
})
11193
} finally {
11294
client.end()
11395
}
11496
} catch (error) {
11597
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
98+
99+
if (isPayloadSizeLimitError(error)) {
100+
logger.warn(`[${requestId}] SSH read file content aborted: ${errorMessage}`)
101+
return NextResponse.json({ error: errorMessage }, { status: 413 })
102+
}
103+
116104
logger.error(`[${requestId}] SSH read file content failed:`, error)
117105

118106
return NextResponse.json(

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import type { Client, SFTPWrapper } from 'ssh2'
66
import { sshWriteFileContentContract } from '@/lib/api/contracts/storage-transfer'
77
import { parseRequest } from '@/lib/api/server'
88
import { checkInternalAuth } from '@/lib/auth/hybrid'
9+
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { MAX_SFTP_READ_BYTES, readSftpFileCapped } from '@/app/api/tools/sftp/utils'
1012
import { createSSHConnection, sanitizePath } from '@/app/api/tools/ssh/utils'
1113

1214
const logger = createLogger('SSHWriteFileContentAPI')
@@ -73,22 +75,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7375
// Handle append mode by reading existing content first
7476
let finalContent = params.content
7577
if (params.mode === 'append') {
76-
const existingContent = await new Promise<string>((resolve) => {
77-
const chunks: Buffer[] = []
78-
const readStream = sftp.createReadStream(filePath)
79-
80-
readStream.on('data', (chunk: Buffer) => {
81-
chunks.push(chunk)
82-
})
83-
84-
readStream.on('end', () => {
85-
resolve(Buffer.concat(chunks).toString('utf-8'))
86-
})
87-
88-
readStream.on('error', () => {
89-
resolve('')
90-
})
91-
})
78+
let existingContent = ''
79+
try {
80+
const existing = await readSftpFileCapped(
81+
sftp,
82+
filePath,
83+
MAX_SFTP_READ_BYTES,
84+
`Existing file '${filePath}'`
85+
)
86+
existingContent = existing.toString('utf-8')
87+
} catch (error) {
88+
if (isPayloadSizeLimitError(error)) throw error
89+
}
9290
finalContent = existingContent + params.content
9391
}
9492

@@ -124,6 +122,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
124122
}
125123
} catch (error) {
126124
const errorMessage = getErrorMessage(error, 'Unknown error occurred')
125+
126+
if (isPayloadSizeLimitError(error)) {
127+
logger.warn(`[${requestId}] SSH write file content aborted: ${errorMessage}`)
128+
return NextResponse.json({ error: errorMessage }, { status: 413 })
129+
}
130+
127131
logger.error(`[${requestId}] SSH write file content failed:`, error)
128132

129133
return NextResponse.json(

0 commit comments

Comments
 (0)