Skip to content

Commit e46048e

Browse files
waleedlatif1aayush598
authored andcommitted
fix(security): allow HTTP for localhost and loopback addresses (simstudioai#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 79b6c26 commit e46048e

4 files changed

Lines changed: 93 additions & 49 deletions

File tree

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
@@ -91,9 +91,9 @@ export function validatePathSegment(
9191
const pathTraversalPatterns = [
9292
'..',
9393
'./',
94-
'.\\.', // Windows path traversal
95-
'%2e%2e', // URL encoded ..
96-
'%252e%252e', // Double URL encoded ..
94+
'.\\.',
95+
'%2e%2e',
96+
'%252e%252e',
9797
'..%2f',
9898
'..%5c',
9999
'%2e%2e%2f',
@@ -393,7 +393,6 @@ export function validateHostname(
393393

394394
const lowerHostname = hostname.toLowerCase()
395395

396-
// Block localhost
397396
if (lowerHostname === 'localhost') {
398397
logger.warn('Hostname is localhost', { paramName })
399398
return {
@@ -402,7 +401,6 @@ export function validateHostname(
402401
}
403402
}
404403

405-
// Use ipaddr.js to check if hostname is an IP and if it's private/reserved
406404
if (ipaddr.isValid(lowerHostname)) {
407405
if (isPrivateOrReservedIP(lowerHostname)) {
408406
logger.warn('Hostname matches blocked IP range', {
@@ -416,7 +414,6 @@ export function validateHostname(
416414
}
417415
}
418416

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

@@ -462,10 +459,7 @@ export function validateFileExtension(
462459
}
463460
}
464461

465-
// Remove leading dot if present
466462
const ext = extension.startsWith('.') ? extension.slice(1) : extension
467-
468-
// Normalize to lowercase
469463
const normalizedExt = ext.toLowerCase()
470464

471465
if (!allowedExtensions.map((e) => e.toLowerCase()).includes(normalizedExt)) {
@@ -517,7 +511,6 @@ export function validateMicrosoftGraphId(
517511
}
518512
}
519513

520-
// Check for path traversal patterns (../)
521514
const pathTraversalPatterns = [
522515
'../',
523516
'..\\',
@@ -527,7 +520,7 @@ export function validateMicrosoftGraphId(
527520
'%2e%2e%5c',
528521
'%2e%2e\\',
529522
'..%5c',
530-
'%252e%252e%252f', // double encoded
523+
'%252e%252e%252f',
531524
]
532525

533526
const lowerValue = value.toLowerCase()
@@ -544,7 +537,6 @@ export function validateMicrosoftGraphId(
544537
}
545538
}
546539

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

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

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

@@ -585,7 +574,6 @@ export function validateJiraCloudId(
585574
value: string | null | undefined,
586575
paramName = 'cloudId'
587576
): ValidationResult {
588-
// Jira cloud IDs are alphanumeric with hyphens (UUID-like)
589577
return validatePathSegment(value, {
590578
paramName,
591579
allowHyphens: true,
@@ -614,7 +602,6 @@ export function validateJiraIssueKey(
614602
value: string | null | undefined,
615603
paramName = 'issueKey'
616604
): ValidationResult {
617-
// Jira issue keys: letters, numbers, hyphens (PROJECT-123 format)
618605
return validatePathSegment(value, {
619606
paramName,
620607
allowHyphens: true,
@@ -655,7 +642,6 @@ export function validateExternalUrl(
655642
}
656643
}
657644

658-
// Must be a valid URL
659645
let parsedUrl: URL
660646
try {
661647
parsedUrl = new URL(url)
@@ -666,28 +652,29 @@ export function validateExternalUrl(
666652
}
667653
}
668654

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

677-
// Block private IP ranges and localhost
678-
const hostname = parsedUrl.hostname.toLowerCase()
679-
680-
// Block localhost
681-
if (hostname === 'localhost') {
669+
if (protocol !== 'https:' && !(protocol === 'http:' && isLocalhost)) {
682670
return {
683671
isValid: false,
684-
error: `${paramName} cannot point to localhost`,
672+
error: `${paramName} must use https:// protocol`,
685673
}
686674
}
687675

688-
// Use ipaddr.js to check if hostname is an IP and if it's private/reserved
689-
if (ipaddr.isValid(hostname)) {
690-
if (isPrivateOrReservedIP(hostname)) {
676+
if (!isLocalhost && ipaddr.isValid(cleanHostname)) {
677+
if (isPrivateOrReservedIP(cleanHostname)) {
691678
return {
692679
isValid: false,
693680
error: `${paramName} cannot point to private IP addresses`,

0 commit comments

Comments
 (0)