Skip to content

Commit bcf65aa

Browse files
committed
path security vuln
1 parent f151650 commit bcf65aa

File tree

6 files changed

+174
-48
lines changed

6 files changed

+174
-48
lines changed

apps/sim/app/api/files/parse/route.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { createLogger } from '@sim/logger'
66
import binaryExtensionsList from 'binary-extensions'
77
import { type NextRequest, NextResponse } from 'next/server'
88
import { checkHybridAuth } from '@/lib/auth/hybrid'
9-
import { validateUrlWithDNS } from '@/lib/core/security/input-validation'
9+
import { secureFetchWithPinnedIP, validateUrlWithDNS } from '@/lib/core/security/input-validation'
1010
import { isSupportedFileType, parseFile } from '@/lib/file-parsers'
1111
import { isUsingCloudStorage, type StorageContext, StorageService } from '@/lib/uploads'
1212
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution'
@@ -349,11 +349,8 @@ async function handleExternalUrl(
349349
}
350350
}
351351

352-
// Use the original URL after DNS validation passes.
353-
// DNS pinning (connecting to IP directly) breaks TLS SNI for HTTPS.
354-
// Since we've validated the IP is not private/reserved, using the original URL is safe.
355-
const response = await fetch(url, {
356-
signal: AbortSignal.timeout(DOWNLOAD_TIMEOUT_MS),
352+
const response = await secureFetchWithPinnedIP(url, urlValidation.resolvedIP!, {
353+
timeout: DOWNLOAD_TIMEOUT_MS,
357354
})
358355
if (!response.ok) {
359356
throw new Error(`Failed to fetch URL: ${response.status} ${response.statusText}`)

apps/sim/app/api/proxy/route.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { z } from 'zod'
55
import { checkHybridAuth } from '@/lib/auth/hybrid'
66
import { generateInternalToken } from '@/lib/auth/internal'
77
import { isDev } from '@/lib/core/config/feature-flags'
8-
import { validateUrlWithDNS } from '@/lib/core/security/input-validation'
8+
import { secureFetchWithPinnedIP, validateUrlWithDNS } from '@/lib/core/security/input-validation'
99
import { generateRequestId } from '@/lib/core/utils/request'
1010
import { getBaseUrl } from '@/lib/core/utils/urls'
1111
import { executeTool } from '@/tools'
@@ -211,15 +211,15 @@ export async function GET(request: Request) {
211211
logger.info(`[${requestId}] Proxying ${method} request to: ${targetUrl}`)
212212

213213
try {
214-
// Use the original URL after DNS validation passes.
215-
// DNS pinning breaks TLS SNI for HTTPS; validation already ensures IP is safe.
216-
const response = await fetch(targetUrl, {
214+
// Use secure fetch with IP pinning to prevent DNS rebinding attacks
215+
// This uses the pre-resolved IP while preserving hostname for TLS SNI
216+
const response = await secureFetchWithPinnedIP(targetUrl, urlValidation.resolvedIP!, {
217217
method: method,
218218
headers: {
219219
...getProxyHeaders(),
220220
...customHeaders,
221221
},
222-
body: body || undefined,
222+
body: body,
223223
})
224224

225225
const contentType = response.headers.get('content-type') || ''
@@ -232,8 +232,8 @@ export async function GET(request: Request) {
232232
}
233233

234234
const errorMessage = !response.ok
235-
? data && typeof data === 'object' && data.error
236-
? `${data.error.message || JSON.stringify(data.error)}`
235+
? data && typeof data === 'object' && (data as { error?: { message?: string } }).error
236+
? `${(data as { error: { message?: string } }).error.message || JSON.stringify((data as { error: unknown }).error)}`
237237
: response.statusText || `HTTP error ${response.status}`
238238
: undefined
239239

@@ -245,7 +245,7 @@ export async function GET(request: Request) {
245245
success: response.ok,
246246
status: response.status,
247247
statusText: response.statusText,
248-
headers: Object.fromEntries(response.headers.entries()),
248+
headers: response.headers.toRecord(),
249249
data,
250250
error: errorMessage,
251251
})

apps/sim/lib/core/security/input-validation.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import dns from 'dns/promises'
2+
import http from 'http'
3+
import https from 'https'
24
import { createLogger } from '@sim/logger'
35

46
const logger = createLogger('InputValidation')
@@ -898,6 +900,139 @@ export function createPinnedUrl(originalUrl: string, resolvedIP: string): string
898900
return `${parsed.protocol}//${host}${port}${parsed.pathname}${parsed.search}`
899901
}
900902

903+
export interface SecureFetchOptions {
904+
method?: string
905+
headers?: Record<string, string>
906+
body?: string
907+
timeout?: number
908+
}
909+
910+
export class SecureFetchHeaders {
911+
private headers: Map<string, string>
912+
913+
constructor(headers: Record<string, string>) {
914+
this.headers = new Map(Object.entries(headers).map(([k, v]) => [k.toLowerCase(), v]))
915+
}
916+
917+
get(name: string): string | null {
918+
return this.headers.get(name.toLowerCase()) ?? null
919+
}
920+
921+
toRecord(): Record<string, string> {
922+
const record: Record<string, string> = {}
923+
for (const [key, value] of this.headers) {
924+
record[key] = value
925+
}
926+
return record
927+
}
928+
929+
[Symbol.iterator]() {
930+
return this.headers.entries()
931+
}
932+
}
933+
934+
export interface SecureFetchResponse {
935+
ok: boolean
936+
status: number
937+
statusText: string
938+
headers: SecureFetchHeaders
939+
text: () => Promise<string>
940+
json: () => Promise<unknown>
941+
arrayBuffer: () => Promise<ArrayBuffer>
942+
}
943+
944+
/**
945+
* Performs a fetch with IP pinning to prevent DNS rebinding attacks.
946+
* Uses the pre-resolved IP address while preserving the original hostname for TLS SNI.
947+
*/
948+
export function secureFetchWithPinnedIP(
949+
url: string,
950+
resolvedIP: string,
951+
options: SecureFetchOptions = {}
952+
): Promise<SecureFetchResponse> {
953+
return new Promise((resolve, reject) => {
954+
const parsed = new URL(url)
955+
const isHttps = parsed.protocol === 'https:'
956+
const defaultPort = isHttps ? 443 : 80
957+
const port = parsed.port ? Number.parseInt(parsed.port, 10) : defaultPort
958+
959+
const isIPv6 = resolvedIP.includes(':')
960+
const family = isIPv6 ? 6 : 4
961+
962+
const agentOptions = {
963+
lookup: (
964+
_hostname: string,
965+
_options: unknown,
966+
callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void
967+
) => {
968+
callback(null, resolvedIP, family)
969+
},
970+
}
971+
972+
const agent = isHttps
973+
? new https.Agent(agentOptions as https.AgentOptions)
974+
: new http.Agent(agentOptions as http.AgentOptions)
975+
976+
const requestOptions: http.RequestOptions = {
977+
hostname: parsed.hostname,
978+
port,
979+
path: parsed.pathname + parsed.search,
980+
method: options.method || 'GET',
981+
headers: options.headers || {},
982+
agent,
983+
timeout: options.timeout || 30000,
984+
}
985+
986+
const protocol = isHttps ? https : http
987+
const req = protocol.request(requestOptions, (res) => {
988+
const chunks: Buffer[] = []
989+
990+
res.on('data', (chunk: Buffer) => chunks.push(chunk))
991+
res.on('end', () => {
992+
const bodyBuffer = Buffer.concat(chunks)
993+
const body = bodyBuffer.toString('utf-8')
994+
const headersRecord: Record<string, string> = {}
995+
for (const [key, value] of Object.entries(res.headers)) {
996+
if (typeof value === 'string') {
997+
headersRecord[key.toLowerCase()] = value
998+
} else if (Array.isArray(value)) {
999+
headersRecord[key.toLowerCase()] = value.join(', ')
1000+
}
1001+
}
1002+
1003+
resolve({
1004+
ok: res.statusCode !== undefined && res.statusCode >= 200 && res.statusCode < 300,
1005+
status: res.statusCode || 0,
1006+
statusText: res.statusMessage || '',
1007+
headers: new SecureFetchHeaders(headersRecord),
1008+
text: async () => body,
1009+
json: async () => JSON.parse(body),
1010+
arrayBuffer: async () =>
1011+
bodyBuffer.buffer.slice(
1012+
bodyBuffer.byteOffset,
1013+
bodyBuffer.byteOffset + bodyBuffer.byteLength
1014+
),
1015+
})
1016+
})
1017+
})
1018+
1019+
req.on('error', (error) => {
1020+
reject(error)
1021+
})
1022+
1023+
req.on('timeout', () => {
1024+
req.destroy()
1025+
reject(new Error('Request timeout'))
1026+
})
1027+
1028+
if (options.body) {
1029+
req.write(options.body)
1030+
}
1031+
1032+
req.end()
1033+
})
1034+
}
1035+
9011036
/**
9021037
* Validates an Airtable ID (base, table, or webhook ID)
9031038
*

apps/sim/lib/uploads/utils/file-utils.server.ts

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use server'
22

33
import type { Logger } from '@sim/logger'
4+
import { secureFetchWithPinnedIP, validateUrlWithDNS } from '@/lib/core/security/input-validation'
45
import type { StorageContext } from '@/lib/uploads'
56
import { isExecutionFile } from '@/lib/uploads/contexts/execution/utils'
67
import { inferContextFromKey } from '@/lib/uploads/utils/file-utils'
@@ -9,38 +10,32 @@ import type { UserFile } from '@/executor/types'
910
/**
1011
* Download a file from a URL (internal or external)
1112
* For internal URLs, uses direct storage access (server-side only)
12-
* For external URLs, uses HTTP fetch
13+
* For external URLs, validates DNS/SSRF and uses secure fetch with IP pinning
1314
*/
1415
export async function downloadFileFromUrl(fileUrl: string, timeoutMs = 180000): Promise<Buffer> {
1516
const { isInternalFileUrl } = await import('./file-utils')
1617
const { parseInternalFileUrl } = await import('./file-utils')
17-
const controller = new AbortController()
18-
const timeoutId = setTimeout(() => controller.abort(), timeoutMs)
1918

20-
try {
21-
if (isInternalFileUrl(fileUrl)) {
22-
const { key, context } = parseInternalFileUrl(fileUrl)
23-
const { downloadFile } = await import('@/lib/uploads/core/storage-service')
24-
const buffer = await downloadFile({ key, context })
25-
clearTimeout(timeoutId)
26-
return buffer
27-
}
19+
if (isInternalFileUrl(fileUrl)) {
20+
const { key, context } = parseInternalFileUrl(fileUrl)
21+
const { downloadFile } = await import('@/lib/uploads/core/storage-service')
22+
return downloadFile({ key, context })
23+
}
2824

29-
const response = await fetch(fileUrl, { signal: controller.signal })
30-
clearTimeout(timeoutId)
25+
const urlValidation = await validateUrlWithDNS(fileUrl, 'fileUrl')
26+
if (!urlValidation.isValid) {
27+
throw new Error(`Invalid file URL: ${urlValidation.error}`)
28+
}
3129

32-
if (!response.ok) {
33-
throw new Error(`Failed to download file: ${response.statusText}`)
34-
}
30+
const response = await secureFetchWithPinnedIP(fileUrl, urlValidation.resolvedIP!, {
31+
timeout: timeoutMs,
32+
})
3533

36-
return Buffer.from(await response.arrayBuffer())
37-
} catch (error) {
38-
clearTimeout(timeoutId)
39-
if (error instanceof Error && error.name === 'AbortError') {
40-
throw new Error('File download timed out')
41-
}
42-
throw error
34+
if (!response.ok) {
35+
throw new Error(`Failed to download file: ${response.statusText}`)
4336
}
37+
38+
return Buffer.from(await response.arrayBuffer())
4439
}
4540

4641
/**

apps/sim/lib/webhooks/rss-polling-service.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { and, eq, isNull, or, sql } from 'drizzle-orm'
55
import { nanoid } from 'nanoid'
66
import Parser from 'rss-parser'
77
import { pollingIdempotency } from '@/lib/core/idempotency/service'
8-
import { validateUrlWithDNS } from '@/lib/core/security/input-validation'
8+
import { secureFetchWithPinnedIP, validateUrlWithDNS } from '@/lib/core/security/input-validation'
99
import { getBaseUrl } from '@/lib/core/utils/urls'
1010
import { MAX_CONSECUTIVE_FAILURES } from '@/triggers/constants'
1111

@@ -265,14 +265,12 @@ async function fetchNewRssItems(
265265
throw new Error(`Invalid RSS feed URL: ${urlValidation.error}`)
266266
}
267267

268-
// Use the original URL after DNS validation passes.
269-
// DNS pinning breaks TLS SNI for HTTPS; validation already ensures IP is safe.
270-
const response = await fetch(config.feedUrl, {
268+
const response = await secureFetchWithPinnedIP(config.feedUrl, urlValidation.resolvedIP!, {
271269
headers: {
272270
'User-Agent': 'Sim/1.0 RSS Poller',
273271
Accept: 'application/rss+xml, application/xml, text/xml, */*',
274272
},
275-
signal: AbortSignal.timeout(30000),
273+
timeout: 30000,
276274
})
277275

278276
if (!response.ok) {

apps/sim/lib/webhooks/utils.server.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import { account, webhook } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { and, eq, isNull, or } from 'drizzle-orm'
55
import { type NextRequest, NextResponse } from 'next/server'
6-
import { validateUrlWithDNS } from '@/lib/core/security/input-validation'
6+
import {
7+
type SecureFetchResponse,
8+
secureFetchWithPinnedIP,
9+
validateUrlWithDNS,
10+
} from '@/lib/core/security/input-validation'
711
import type { DbOrTx } from '@/lib/db/types'
812
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
913

@@ -98,7 +102,7 @@ async function fetchWithDNSPinning(
98102
url: string,
99103
accessToken: string,
100104
requestId: string
101-
): Promise<Response | null> {
105+
): Promise<SecureFetchResponse | null> {
102106
try {
103107
const urlValidation = await validateUrlWithDNS(url, 'contentUrl')
104108
if (!urlValidation.isValid) {
@@ -108,17 +112,14 @@ async function fetchWithDNSPinning(
108112
return null
109113
}
110114

111-
// Use the original URL after DNS validation passes.
112-
// DNS pinning breaks TLS SNI for HTTPS; validation already ensures IP is safe.
113115
const headers: Record<string, string> = {}
114116

115117
if (accessToken) {
116118
headers.Authorization = `Bearer ${accessToken}`
117119
}
118120

119-
const response = await fetch(url, {
121+
const response = await secureFetchWithPinnedIP(url, urlValidation.resolvedIP!, {
120122
headers,
121-
redirect: 'follow',
122123
})
123124

124125
return response

0 commit comments

Comments
 (0)