Skip to content

Commit 996dc96

Browse files
fix(security): allow HTTP for localhost and loopback addresses (#3304)
* fix(security): allow localhost HTTP without weakening SSRF protections * fix(security): remove extraneous comments and fix failing SSRF test * fix(security): derive isLocalhost from hostname not resolved IP in validateUrlWithDNS * fix(security): verify resolved IP is loopback when hostname is localhost in validateUrlWithDNS --------- Co-authored-by: aayush598 <aayushgid598@gmail.com>
1 parent 04286fc commit 996dc96

File tree

4 files changed

+93
-49
lines changed

4 files changed

+93
-49
lines changed

apps/sim/app/api/function/execute/route.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ describe('Function Execute API Route', () => {
211211

212212
it.concurrent('should block SSRF attacks through secure fetch wrapper', async () => {
213213
expect(validateProxyUrl('http://169.254.169.254/latest/meta-data/').isValid).toBe(false)
214-
expect(validateProxyUrl('http://127.0.0.1:8080/admin').isValid).toBe(false)
214+
expect(validateProxyUrl('http://127.0.0.1:8080/admin').isValid).toBe(true)
215215
expect(validateProxyUrl('http://192.168.1.1/config').isValid).toBe(false)
216216
expect(validateProxyUrl('http://10.0.0.1/internal').isValid).toBe(false)
217217
})

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

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,31 @@ export async function validateUrlWithDNS(
6464
const parsedUrl = new URL(url!)
6565
const hostname = parsedUrl.hostname
6666

67+
const hostnameLower = hostname.toLowerCase()
68+
const cleanHostname =
69+
hostnameLower.startsWith('[') && hostnameLower.endsWith(']')
70+
? hostnameLower.slice(1, -1)
71+
: hostnameLower
72+
73+
let isLocalhost = cleanHostname === 'localhost'
74+
if (ipaddr.isValid(cleanHostname)) {
75+
const processedIP = ipaddr.process(cleanHostname).toString()
76+
if (processedIP === '127.0.0.1' || processedIP === '::1') {
77+
isLocalhost = true
78+
}
79+
}
80+
6781
try {
68-
const { address } = await dns.lookup(hostname)
82+
const { address } = await dns.lookup(cleanHostname, { verbatim: true })
83+
84+
const resolvedIsLoopback =
85+
ipaddr.isValid(address) &&
86+
(() => {
87+
const ip = ipaddr.process(address).toString()
88+
return ip === '127.0.0.1' || ip === '::1'
89+
})()
6990

70-
if (isPrivateOrReservedIP(address)) {
91+
if (isPrivateOrReservedIP(address) && !(isLocalhost && resolvedIsLoopback)) {
7192
logger.warn('URL resolves to blocked IP address', {
7293
paramName,
7394
hostname,
@@ -189,8 +210,6 @@ export async function secureFetchWithPinnedIP(
189210

190211
const agent = isHttps ? new https.Agent(agentOptions) : new http.Agent(agentOptions)
191212

192-
// Remove accept-encoding since Node.js http/https doesn't auto-decompress
193-
// Headers are lowercase due to Web Headers API normalization in executeToolRequest
194213
const { 'accept-encoding': _, ...sanitizedHeaders } = options.headers ?? {}
195214

196215
const requestOptions: http.RequestOptions = {
@@ -200,7 +219,7 @@ export async function secureFetchWithPinnedIP(
200219
method: options.method || 'GET',
201220
headers: sanitizedHeaders,
202221
agent,
203-
timeout: options.timeout || 300000, // Default 5 minutes
222+
timeout: options.timeout || 300000,
204223
}
205224

206225
const protocol = isHttps ? https : http

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

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -569,10 +569,28 @@ describe('validateUrlWithDNS', () => {
569569
expect(result.error).toContain('https://')
570570
})
571571

572-
it('should reject localhost URLs', async () => {
572+
it('should accept https localhost URLs', async () => {
573573
const result = await validateUrlWithDNS('https://localhost/api')
574-
expect(result.isValid).toBe(false)
575-
expect(result.error).toContain('localhost')
574+
expect(result.isValid).toBe(true)
575+
expect(result.resolvedIP).toBeDefined()
576+
})
577+
578+
it('should accept http localhost URLs', async () => {
579+
const result = await validateUrlWithDNS('http://localhost/api')
580+
expect(result.isValid).toBe(true)
581+
expect(result.resolvedIP).toBeDefined()
582+
})
583+
584+
it('should accept IPv4 loopback URLs', async () => {
585+
const result = await validateUrlWithDNS('http://127.0.0.1/api')
586+
expect(result.isValid).toBe(true)
587+
expect(result.resolvedIP).toBeDefined()
588+
})
589+
590+
it('should accept IPv6 loopback URLs', async () => {
591+
const result = await validateUrlWithDNS('http://[::1]/api')
592+
expect(result.isValid).toBe(true)
593+
expect(result.resolvedIP).toBeDefined()
576594
})
577595

578596
it('should reject private IP URLs', async () => {
@@ -898,17 +916,37 @@ describe('validateExternalUrl', () => {
898916
expect(result.isValid).toBe(false)
899917
expect(result.error).toContain('valid URL')
900918
})
919+
})
901920

902-
it.concurrent('should reject localhost', () => {
921+
describe('localhost and loopback addresses', () => {
922+
it.concurrent('should accept https localhost', () => {
903923
const result = validateExternalUrl('https://localhost/api')
904-
expect(result.isValid).toBe(false)
905-
expect(result.error).toContain('localhost')
924+
expect(result.isValid).toBe(true)
906925
})
907926

908-
it.concurrent('should reject 127.0.0.1', () => {
927+
it.concurrent('should accept http localhost', () => {
928+
const result = validateExternalUrl('http://localhost/api')
929+
expect(result.isValid).toBe(true)
930+
})
931+
932+
it.concurrent('should accept https 127.0.0.1', () => {
909933
const result = validateExternalUrl('https://127.0.0.1/api')
910-
expect(result.isValid).toBe(false)
911-
expect(result.error).toContain('private IP')
934+
expect(result.isValid).toBe(true)
935+
})
936+
937+
it.concurrent('should accept http 127.0.0.1', () => {
938+
const result = validateExternalUrl('http://127.0.0.1/api')
939+
expect(result.isValid).toBe(true)
940+
})
941+
942+
it.concurrent('should accept https IPv6 loopback', () => {
943+
const result = validateExternalUrl('https://[::1]/api')
944+
expect(result.isValid).toBe(true)
945+
})
946+
947+
it.concurrent('should accept http IPv6 loopback', () => {
948+
const result = validateExternalUrl('http://[::1]/api')
949+
expect(result.isValid).toBe(true)
912950
})
913951

914952
it.concurrent('should reject 0.0.0.0', () => {
@@ -989,9 +1027,9 @@ describe('validateImageUrl', () => {
9891027
expect(result.isValid).toBe(true)
9901028
})
9911029

992-
it.concurrent('should reject localhost URLs', () => {
1030+
it.concurrent('should accept localhost URLs', () => {
9931031
const result = validateImageUrl('https://localhost/image.png')
994-
expect(result.isValid).toBe(false)
1032+
expect(result.isValid).toBe(true)
9951033
})
9961034

9971035
it.concurrent('should use imageUrl as default param name', () => {

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

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ export function validatePathSegment(
8989
const pathTraversalPatterns = [
9090
'..',
9191
'./',
92-
'.\\.', // Windows path traversal
93-
'%2e%2e', // URL encoded ..
94-
'%252e%252e', // Double URL encoded ..
92+
'.\\.',
93+
'%2e%2e',
94+
'%252e%252e',
9595
'..%2f',
9696
'..%5c',
9797
'%2e%2e%2f',
@@ -391,7 +391,6 @@ export function validateHostname(
391391

392392
const lowerHostname = hostname.toLowerCase()
393393

394-
// Block localhost
395394
if (lowerHostname === 'localhost') {
396395
logger.warn('Hostname is localhost', { paramName })
397396
return {
@@ -400,7 +399,6 @@ export function validateHostname(
400399
}
401400
}
402401

403-
// Use ipaddr.js to check if hostname is an IP and if it's private/reserved
404402
if (ipaddr.isValid(lowerHostname)) {
405403
if (isPrivateOrReservedIP(lowerHostname)) {
406404
logger.warn('Hostname matches blocked IP range', {
@@ -414,7 +412,6 @@ export function validateHostname(
414412
}
415413
}
416414

417-
// Basic hostname format validation
418415
const hostnamePattern =
419416
/^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$/i
420417

@@ -460,10 +457,7 @@ export function validateFileExtension(
460457
}
461458
}
462459

463-
// Remove leading dot if present
464460
const ext = extension.startsWith('.') ? extension.slice(1) : extension
465-
466-
// Normalize to lowercase
467461
const normalizedExt = ext.toLowerCase()
468462

469463
if (!allowedExtensions.map((e) => e.toLowerCase()).includes(normalizedExt)) {
@@ -515,7 +509,6 @@ export function validateMicrosoftGraphId(
515509
}
516510
}
517511

518-
// Check for path traversal patterns (../)
519512
const pathTraversalPatterns = [
520513
'../',
521514
'..\\',
@@ -525,7 +518,7 @@ export function validateMicrosoftGraphId(
525518
'%2e%2e%5c',
526519
'%2e%2e\\',
527520
'..%5c',
528-
'%252e%252e%252f', // double encoded
521+
'%252e%252e%252f',
529522
]
530523

531524
const lowerValue = value.toLowerCase()
@@ -542,7 +535,6 @@ export function validateMicrosoftGraphId(
542535
}
543536
}
544537

545-
// Check for control characters and null bytes
546538
if (/[\x00-\x1f\x7f]/.test(value) || value.includes('%00')) {
547539
logger.warn('Control characters in Microsoft Graph ID', { paramName })
548540
return {
@@ -551,16 +543,13 @@ export function validateMicrosoftGraphId(
551543
}
552544
}
553545

554-
// Check for newlines (which could be used for header injection)
555546
if (value.includes('\n') || value.includes('\r')) {
556547
return {
557548
isValid: false,
558549
error: `${paramName} contains invalid newline characters`,
559550
}
560551
}
561552

562-
// Microsoft Graph IDs can contain many characters, but not suspicious patterns
563-
// We've blocked path traversal, so allow the rest
564553
return { isValid: true, sanitized: value }
565554
}
566555

@@ -583,7 +572,6 @@ export function validateJiraCloudId(
583572
value: string | null | undefined,
584573
paramName = 'cloudId'
585574
): ValidationResult {
586-
// Jira cloud IDs are alphanumeric with hyphens (UUID-like)
587575
return validatePathSegment(value, {
588576
paramName,
589577
allowHyphens: true,
@@ -612,7 +600,6 @@ export function validateJiraIssueKey(
612600
value: string | null | undefined,
613601
paramName = 'issueKey'
614602
): ValidationResult {
615-
// Jira issue keys: letters, numbers, hyphens (PROJECT-123 format)
616603
return validatePathSegment(value, {
617604
paramName,
618605
allowHyphens: true,
@@ -653,7 +640,6 @@ export function validateExternalUrl(
653640
}
654641
}
655642

656-
// Must be a valid URL
657643
let parsedUrl: URL
658644
try {
659645
parsedUrl = new URL(url)
@@ -664,28 +650,29 @@ export function validateExternalUrl(
664650
}
665651
}
666652

667-
// Only allow https protocol
668-
if (parsedUrl.protocol !== 'https:') {
669-
return {
670-
isValid: false,
671-
error: `${paramName} must use https:// protocol`,
653+
const protocol = parsedUrl.protocol
654+
const hostname = parsedUrl.hostname.toLowerCase()
655+
656+
const cleanHostname =
657+
hostname.startsWith('[') && hostname.endsWith(']') ? hostname.slice(1, -1) : hostname
658+
659+
let isLocalhost = cleanHostname === 'localhost'
660+
if (ipaddr.isValid(cleanHostname)) {
661+
const processedIP = ipaddr.process(cleanHostname).toString()
662+
if (processedIP === '127.0.0.1' || processedIP === '::1') {
663+
isLocalhost = true
672664
}
673665
}
674666

675-
// Block private IP ranges and localhost
676-
const hostname = parsedUrl.hostname.toLowerCase()
677-
678-
// Block localhost
679-
if (hostname === 'localhost') {
667+
if (protocol !== 'https:' && !(protocol === 'http:' && isLocalhost)) {
680668
return {
681669
isValid: false,
682-
error: `${paramName} cannot point to localhost`,
670+
error: `${paramName} must use https:// protocol`,
683671
}
684672
}
685673

686-
// Use ipaddr.js to check if hostname is an IP and if it's private/reserved
687-
if (ipaddr.isValid(hostname)) {
688-
if (isPrivateOrReservedIP(hostname)) {
674+
if (!isLocalhost && ipaddr.isValid(cleanHostname)) {
675+
if (isPrivateOrReservedIP(cleanHostname)) {
689676
return {
690677
isValid: false,
691678
error: `${paramName} cannot point to private IP addresses`,

0 commit comments

Comments
 (0)