Skip to content

Commit d89ab4a

Browse files
authored
fix(security): bound response bodies in secureFetchWithPinnedIP by default (#6169)
secureFetchWithPinnedIP only capped a response body when the caller passed maxResponseBytes; with the option absent the body streamed into an unbounded Buffer.concat. Several tool proxies whose target host is user-supplied (Jupyter, ClickHouse, Grafana, 1Password Connect) and the RSS poller called it without that option, so an attacker-controlled server answering with an endless chunked body could grow the shared process heap until it was OOM-killed. Make the cap fail-safe: default to 100MB (and treat a non-positive value as the default) so there is no unlimited mode, then pass tighter explicit caps at the user-supplied-host sites. Responses that carry no body (HEAD, 204, 304) are exempted from the content-length pre-check — they advertise the resource size as metadata, which would otherwise spuriously fail a HEAD probe of a large file or an RSS conditional-GET 304.
1 parent 47e8f1e commit d89ab4a

13 files changed

Lines changed: 183 additions & 9 deletions

File tree

apps/sim/app/api/tools/clickhouse/utils.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { mockValidateDatabaseHost, mockSecureFetchWithPinnedIP, mockValidateSqlWh
1212
}))
1313

1414
vi.mock('@/lib/core/security/input-validation.server', () => ({
15+
MAX_JSON_API_RESPONSE_BYTES: 10 * 1024 * 1024,
1516
validateDatabaseHost: mockValidateDatabaseHost,
1617
secureFetchWithPinnedIP: mockSecureFetchWithPinnedIP,
1718
validateSqlWhereClause: mockValidateSqlWhereClause,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
MAX_JSON_API_RESPONSE_BYTES,
23
secureFetchWithPinnedIP,
34
validateDatabaseHost,
45
validateSqlWhereClause,
@@ -96,6 +97,7 @@ async function clickhouseRequest(
9697
body: statement,
9798
timeout: REQUEST_TIMEOUT_MS,
9899
allowHttp: !config.secure,
100+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
99101
})
100102

101103
const text = await response.text()

apps/sim/app/api/tools/grafana/update_alert_rule/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { grafanaUpdateAlertRuleContract } from '@/lib/api/contracts/tools/grafan
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import {
8+
MAX_JSON_API_RESPONSE_BYTES,
89
secureFetchWithPinnedIP,
910
validateUrlWithDNS,
1011
} from '@/lib/core/security/input-validation.server'
@@ -76,6 +77,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7677
const getResponse = await secureFetchWithPinnedIP(getUrl, getValidation.resolvedIP, {
7778
method: 'GET',
7879
headers: getHeaders,
80+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
7981
})
8082

8183
if (!getResponse.ok) {
@@ -205,6 +207,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
205207
method: 'PUT',
206208
headers,
207209
body: JSON.stringify(updatedRule),
210+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
208211
})
209212

210213
if (!updateResponse.ok) {

apps/sim/app/api/tools/grafana/update_dashboard/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { grafanaUpdateDashboardContract } from '@/lib/api/contracts/tools/grafan
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import {
8+
MAX_JSON_API_RESPONSE_BYTES,
89
secureFetchWithPinnedIP,
910
validateUrlWithDNS,
1011
} from '@/lib/core/security/input-validation.server'
@@ -75,6 +76,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7576
const getResponse = await secureFetchWithPinnedIP(getUrl, getValidation.resolvedIP, {
7677
method: 'GET',
7778
headers: getHeaders,
79+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
7880
})
7981

8082
if (!getResponse.ok) {
@@ -166,6 +168,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
166168
method: 'POST',
167169
headers,
168170
body: JSON.stringify(body),
171+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
169172
})
170173

171174
if (!updateResponse.ok) {

apps/sim/app/api/tools/grafana/update_folder/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { grafanaUpdateFolderContract } from '@/lib/api/contracts/tools/grafana'
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import {
8+
MAX_JSON_API_RESPONSE_BYTES,
89
secureFetchWithPinnedIP,
910
validateUrlWithDNS,
1011
} from '@/lib/core/security/input-validation.server'
@@ -73,6 +74,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7374
const getResponse = await secureFetchWithPinnedIP(folderUrl, urlValidation.resolvedIP, {
7475
method: 'GET',
7576
headers,
77+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
7678
})
7779

7880
if (!getResponse.ok) {
@@ -104,6 +106,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
104106
method: 'PUT',
105107
headers,
106108
body: JSON.stringify(body),
109+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
107110
})
108111

109112
if (!updateResponse.ok) {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { jupyterProxyContract } from '@/lib/api/contracts/tools/jupyter'
55
import { parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import {
8+
MAX_JSON_API_RESPONSE_BYTES,
89
secureFetchWithPinnedIP,
910
validateUrlWithDNS,
1011
} from '@/lib/core/security/input-validation.server'
@@ -77,6 +78,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7778
body: hasBody ? JSON.stringify(data.body) : undefined,
7879
allowHttp: true,
7980
maxRedirects: 0,
81+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
8082
})
8183

8284
const text = await upstream.text()

apps/sim/app/api/tools/jupyter/upload/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { jupyterUploadContract } from '@/lib/api/contracts/storage-transfer'
55
import { parseRequest } from '@/lib/api/server'
66
import { checkInternalAuth } from '@/lib/auth/hybrid'
77
import {
8+
MAX_JSON_API_RESPONSE_BYTES,
89
secureFetchWithPinnedIP,
910
validateUrlWithDNS,
1011
} from '@/lib/core/security/input-validation.server'
@@ -118,6 +119,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
118119
}),
119120
allowHttp: true,
120121
maxRedirects: 0,
122+
maxResponseBytes: MAX_JSON_API_RESPONSE_BYTES,
121123
})
122124

123125
if (!response.ok) {

apps/sim/app/api/tools/onepassword/get-item-file/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { onePasswordGetItemFileContract } from '@/lib/api/contracts/tools/onepas
66
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
77
import { checkInternalAuth } from '@/lib/auth/hybrid'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
9+
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
910
import {
1011
connectRequest,
1112
createOnePasswordClient,
@@ -80,6 +81,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
8081
apiKey: creds.apiKey!,
8182
path: `/v1/vaults/${params.vaultId}/items/${params.itemId}/files/${params.fileId}/content`,
8283
method: 'GET',
84+
maxResponseBytes: MAX_FILE_SIZE,
8385
})
8486
if (!contentResponse.ok) {
8587
const errorData = await contentResponse.json().catch(() => ({}))

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import { toError } from '@sim/utils/errors'
1717
import { generateId } from '@sim/utils/id'
1818
import * as ipaddr from 'ipaddr.js'
1919
import { isHosted } from '@/lib/core/config/env-flags'
20-
import { secureFetchWithPinnedIP } from '@/lib/core/security/input-validation.server'
20+
import {
21+
MAX_JSON_API_RESPONSE_BYTES,
22+
secureFetchWithPinnedIP,
23+
} from '@/lib/core/security/input-validation.server'
2124

2225
/** Connect-format field type strings returned by normalization. */
2326
type ConnectFieldType =
@@ -344,14 +347,21 @@ export interface ConnectResponse {
344347
arrayBuffer: () => Promise<ArrayBuffer>
345348
}
346349

347-
/** Proxy a request to the 1Password Connect Server. */
350+
/**
351+
* Proxy a request to the 1Password Connect Server.
352+
*
353+
* The Connect server is self-hosted at a user-supplied `serverUrl`, so the response body
354+
* is always capped. JSON endpoints use {@link MAX_JSON_API_RESPONSE_BYTES}; callers
355+
* downloading file content pass a larger `maxResponseBytes` explicitly.
356+
*/
348357
export async function connectRequest(options: {
349358
serverUrl: string
350359
apiKey: string
351360
path: string
352361
method: string
353362
body?: unknown
354363
query?: string
364+
maxResponseBytes?: number
355365
}): Promise<ConnectResponse> {
356366
const resolvedIP = await validateConnectServerUrl(options.serverUrl)
357367

@@ -372,6 +382,7 @@ export async function connectRequest(options: {
372382
headers,
373383
body: options.body ? JSON.stringify(options.body) : undefined,
374384
allowHttp: true,
385+
maxResponseBytes: options.maxResponseBytes ?? MAX_JSON_API_RESPONSE_BYTES,
375386
})
376387
}
377388

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

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,11 @@ export interface SecureFetchOptions {
358358
body?: string | Buffer | Uint8Array
359359
timeout?: number
360360
maxRedirects?: number
361+
/**
362+
* Maximum bytes read from the response body. Defaults to
363+
* {@link DEFAULT_MAX_RESPONSE_BYTES} — there is deliberately no "unlimited" mode, since
364+
* many callers target a user-supplied host that can stream an endless body.
365+
*/
361366
maxResponseBytes?: number
362367
signal?: AbortSignal
363368
/** Drop the Authorization header when following a redirect, so it is not sent to the redirect target's origin. */
@@ -414,6 +419,19 @@ export interface SecureFetchResponse {
414419

415420
const DEFAULT_MAX_REDIRECTS = 5
416421

422+
/**
423+
* Fail-safe ceiling applied by {@link secureFetchWithPinnedIP} when the caller does not
424+
* pass `maxResponseBytes`. Many callers fetch a user-supplied host, so an omitted cap
425+
* would let a malicious upstream stream an endless chunked body into memory until the
426+
* process is OOM-killed. Set to the platform's largest legitimate payload (100MB, matching
427+
* the upload limit); callers that need more must opt in explicitly, and callers handling
428+
* small JSON should pass a much tighter cap.
429+
*/
430+
export const DEFAULT_MAX_RESPONSE_BYTES = 100 * 1024 * 1024
431+
432+
/** Response cap for JSON/control-plane proxies to user-supplied hosts. */
433+
export const MAX_JSON_API_RESPONSE_BYTES = 10 * 1024 * 1024
434+
417435
function isRedirectStatus(status: number): boolean {
418436
return status >= 300 && status < 400 && status !== 304
419437
}
@@ -937,6 +955,10 @@ export function createPinnedFetchWithDispatcher(
937955
* Performs a fetch with IP pinning to prevent DNS rebinding attacks.
938956
* Uses the pre-resolved IP address while preserving the original hostname for TLS SNI.
939957
* Follows redirects securely by validating each redirect target.
958+
*
959+
* The response body is always bounded — `options.maxResponseBytes` when supplied (and
960+
* positive), otherwise {@link DEFAULT_MAX_RESPONSE_BYTES}. Exceeding the cap rejects with
961+
* a {@link PayloadSizeLimitError} and destroys the socket.
940962
*/
941963
export async function secureFetchWithPinnedIP(
942964
url: string,
@@ -945,7 +967,11 @@ export async function secureFetchWithPinnedIP(
945967
redirectCount = 0
946968
): Promise<SecureFetchResponse> {
947969
const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS
948-
const maxResponseBytes = options.maxResponseBytes
970+
const requestedMaxResponseBytes = options.maxResponseBytes
971+
const maxResponseBytes =
972+
typeof requestedMaxResponseBytes === 'number' && requestedMaxResponseBytes > 0
973+
? requestedMaxResponseBytes
974+
: DEFAULT_MAX_RESPONSE_BYTES
949975

950976
return new Promise((resolve, reject) => {
951977
const parsed = new URL(url)
@@ -1037,8 +1063,15 @@ export async function secureFetchWithPinnedIP(
10371063
}
10381064
}
10391065

1066+
// Responses that carry no body (HEAD, 204, 304) may still advertise the resource's full
1067+
// size in content-length. That is metadata, not a payload, so it must not trip the cap —
1068+
// otherwise a HEAD probe of a large file, or a conditional-GET 304, would fail spuriously.
1069+
const isBodylessResponse =
1070+
(requestOptions.method || 'GET').toUpperCase() === 'HEAD' ||
1071+
statusCode === 204 ||
1072+
statusCode === 304
10401073
const contentLength = headersRecord['content-length']
1041-
if (typeof maxResponseBytes === 'number' && maxResponseBytes > 0 && contentLength) {
1074+
if (contentLength && !isBodylessResponse) {
10421075
const parsedLength = Number.parseInt(contentLength, 10)
10431076
if (Number.isFinite(parsedLength) && parsedLength > maxResponseBytes) {
10441077
cleanupAbort()
@@ -1074,11 +1107,7 @@ export async function secureFetchWithPinnedIP(
10741107
start(controller) {
10751108
nodeRes.on('data', (chunk: Buffer) => {
10761109
totalBytes += chunk.length
1077-
if (
1078-
typeof maxResponseBytes === 'number' &&
1079-
maxResponseBytes > 0 &&
1080-
totalBytes > maxResponseBytes
1081-
) {
1110+
if (totalBytes > maxResponseBytes) {
10821111
cleanupAbort()
10831112
controller.error(
10841113
new PayloadSizeLimitError({

0 commit comments

Comments
 (0)