mirror of
https://github.com/simstudioai/sim.git
synced 2026-04-06 03:00:16 -04:00
fix(security): fix ssrf vuln and path validation for files route (#1325)
* update infra and remove railway
* fix(security): fix ssrf vuln
* fix path validation for file serve
* Revert "update infra and remove railway"
This reverts commit abfa2f8d51.
* lint
* ack PR comments
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { createFileResponse, extractFilename } from './utils'
|
||||
import { createFileResponse, extractFilename, findLocalFile } from './utils'
|
||||
|
||||
describe('extractFilename', () => {
|
||||
describe('legitimate file paths', () => {
|
||||
@@ -325,3 +325,91 @@ describe('extractFilename', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('findLocalFile - Path Traversal Security Tests', () => {
|
||||
describe('path traversal attack prevention', () => {
|
||||
it.concurrent('should reject classic path traversal attacks', () => {
|
||||
const maliciousInputs = [
|
||||
'../../../etc/passwd',
|
||||
'..\\..\\..\\windows\\system32\\config\\sam',
|
||||
'../../../../etc/shadow',
|
||||
'../config.json',
|
||||
'..\\config.ini',
|
||||
]
|
||||
|
||||
maliciousInputs.forEach((input) => {
|
||||
const result = findLocalFile(input)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it.concurrent('should reject encoded path traversal attempts', () => {
|
||||
const encodedInputs = [
|
||||
'%2e%2e%2f%2e%2e%2f%65%74%63%2f%70%61%73%73%77%64', // ../../../etc/passwd
|
||||
'..%2f..%2fetc%2fpasswd',
|
||||
'..%5c..%5cconfig.ini',
|
||||
]
|
||||
|
||||
encodedInputs.forEach((input) => {
|
||||
const result = findLocalFile(input)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it.concurrent('should reject mixed path separators', () => {
|
||||
const mixedInputs = ['../..\\config.txt', '..\\../secret.ini', '/..\\..\\system32']
|
||||
|
||||
mixedInputs.forEach((input) => {
|
||||
const result = findLocalFile(input)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it.concurrent('should reject filenames with dangerous characters', () => {
|
||||
const dangerousInputs = [
|
||||
'file:with:colons.txt',
|
||||
'file|with|pipes.txt',
|
||||
'file?with?questions.txt',
|
||||
'file*with*asterisks.txt',
|
||||
]
|
||||
|
||||
dangerousInputs.forEach((input) => {
|
||||
const result = findLocalFile(input)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it.concurrent('should reject null and empty inputs', () => {
|
||||
expect(findLocalFile('')).toBeNull()
|
||||
expect(findLocalFile(' ')).toBeNull()
|
||||
expect(findLocalFile('\t\n')).toBeNull()
|
||||
})
|
||||
|
||||
it.concurrent('should reject filenames that become empty after sanitization', () => {
|
||||
const emptyAfterSanitization = ['../..', '..\\..\\', '////', '....', '..']
|
||||
|
||||
emptyAfterSanitization.forEach((input) => {
|
||||
const result = findLocalFile(input)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('security validation passes for legitimate files', () => {
|
||||
it.concurrent('should accept properly formatted filenames without throwing errors', () => {
|
||||
const legitimateInputs = [
|
||||
'document.pdf',
|
||||
'image.png',
|
||||
'data.csv',
|
||||
'report-2024.doc',
|
||||
'file_with_underscores.txt',
|
||||
'file-with-dashes.json',
|
||||
]
|
||||
|
||||
legitimateInputs.forEach((input) => {
|
||||
// Should not throw security errors for legitimate filenames
|
||||
expect(() => findLocalFile(input)).not.toThrow()
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
import { existsSync } from 'fs'
|
||||
import { join } from 'path'
|
||||
import { join, resolve, sep } from 'path'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { createLogger } from '@/lib/logs/console/logger'
|
||||
import { UPLOAD_DIR } from '@/lib/uploads/setup'
|
||||
|
||||
const logger = createLogger('FilesUtils')
|
||||
|
||||
/**
|
||||
* Response type definitions
|
||||
*/
|
||||
@@ -192,18 +195,71 @@ export function extractFilename(path: string): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a file in possible local storage locations
|
||||
* Sanitize filename to prevent path traversal attacks
|
||||
*/
|
||||
export function findLocalFile(filename: string): string | null {
|
||||
const possiblePaths = [join(UPLOAD_DIR, filename), join(process.cwd(), 'uploads', filename)]
|
||||
|
||||
for (const path of possiblePaths) {
|
||||
if (existsSync(path)) {
|
||||
return path
|
||||
}
|
||||
function sanitizeFilename(filename: string): string {
|
||||
if (!filename || typeof filename !== 'string') {
|
||||
throw new Error('Invalid filename provided')
|
||||
}
|
||||
|
||||
return null
|
||||
const sanitized = filename
|
||||
.replace(/\.\./g, '') // Remove .. sequences
|
||||
.replace(/[/\\]/g, '') // Remove path separators
|
||||
.replace(/^\./g, '') // Remove leading dots
|
||||
.trim()
|
||||
|
||||
if (!sanitized || sanitized.length === 0) {
|
||||
throw new Error('Invalid or empty filename after sanitization')
|
||||
}
|
||||
|
||||
if (
|
||||
sanitized.includes(':') ||
|
||||
sanitized.includes('|') ||
|
||||
sanitized.includes('?') ||
|
||||
sanitized.includes('*') ||
|
||||
sanitized.includes('\x00') || // Null bytes
|
||||
/[\x00-\x1F\x7F]/.test(sanitized) // Control characters
|
||||
) {
|
||||
throw new Error('Filename contains invalid characters')
|
||||
}
|
||||
|
||||
return sanitized
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a file in possible local storage locations with proper path validation
|
||||
*/
|
||||
export function findLocalFile(filename: string): string | null {
|
||||
try {
|
||||
const sanitizedFilename = sanitizeFilename(filename)
|
||||
|
||||
const possiblePaths = [
|
||||
join(UPLOAD_DIR, sanitizedFilename),
|
||||
join(process.cwd(), 'uploads', sanitizedFilename),
|
||||
]
|
||||
|
||||
for (const path of possiblePaths) {
|
||||
const resolvedPath = resolve(path)
|
||||
const allowedDirs = [resolve(UPLOAD_DIR), resolve(process.cwd(), 'uploads')]
|
||||
|
||||
const isWithinAllowedDir = allowedDirs.some(
|
||||
(allowedDir) => resolvedPath.startsWith(allowedDir + sep) || resolvedPath === allowedDir
|
||||
)
|
||||
|
||||
if (!isWithinAllowedDir) {
|
||||
continue // Skip this path as it's outside allowed directories
|
||||
}
|
||||
|
||||
if (existsSync(resolvedPath)) {
|
||||
return resolvedPath
|
||||
}
|
||||
}
|
||||
|
||||
return null
|
||||
} catch (error) {
|
||||
logger.error('Error in findLocalFile:', error)
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
const SAFE_INLINE_TYPES = new Set([
|
||||
|
||||
@@ -48,8 +48,52 @@ describe('Function Execute API Route', () => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('Security Tests', () => {
|
||||
it.concurrent('should create secure fetch in VM context', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "test"',
|
||||
useLocalVM: true,
|
||||
})
|
||||
|
||||
const { POST } = await import('@/app/api/function/execute/route')
|
||||
await POST(req)
|
||||
|
||||
expect(mockCreateContext).toHaveBeenCalled()
|
||||
const contextArgs = mockCreateContext.mock.calls[0][0]
|
||||
expect(contextArgs).toHaveProperty('fetch')
|
||||
expect(typeof contextArgs.fetch).toBe('function')
|
||||
|
||||
expect(contextArgs.fetch.name).toBe('secureFetch')
|
||||
})
|
||||
|
||||
it.concurrent('should block SSRF attacks through secure fetch wrapper', async () => {
|
||||
const { validateProxyUrl } = await import('@/lib/security/url-validation')
|
||||
|
||||
expect(validateProxyUrl('http://169.254.169.254/latest/meta-data/').isValid).toBe(false)
|
||||
expect(validateProxyUrl('http://127.0.0.1:8080/admin').isValid).toBe(false)
|
||||
expect(validateProxyUrl('http://192.168.1.1/config').isValid).toBe(false)
|
||||
expect(validateProxyUrl('http://10.0.0.1/internal').isValid).toBe(false)
|
||||
})
|
||||
|
||||
it.concurrent('should allow legitimate external URLs', async () => {
|
||||
const { validateProxyUrl } = await import('@/lib/security/url-validation')
|
||||
|
||||
expect(validateProxyUrl('https://api.github.com/user').isValid).toBe(true)
|
||||
expect(validateProxyUrl('https://httpbin.org/get').isValid).toBe(true)
|
||||
expect(validateProxyUrl('http://example.com/api').isValid).toBe(true)
|
||||
})
|
||||
|
||||
it.concurrent('should block dangerous protocols', async () => {
|
||||
const { validateProxyUrl } = await import('@/lib/security/url-validation')
|
||||
|
||||
expect(validateProxyUrl('file:///etc/passwd').isValid).toBe(false)
|
||||
expect(validateProxyUrl('ftp://internal.server/files').isValid).toBe(false)
|
||||
expect(validateProxyUrl('gopher://old.server/menu').isValid).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Basic Function Execution', () => {
|
||||
it('should execute simple JavaScript code successfully', async () => {
|
||||
it.concurrent('should execute simple JavaScript code successfully', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "Hello World"',
|
||||
timeout: 5000,
|
||||
@@ -66,7 +110,7 @@ describe('Function Execute API Route', () => {
|
||||
expect(data.output).toHaveProperty('executionTime')
|
||||
})
|
||||
|
||||
it('should handle missing code parameter', async () => {
|
||||
it.concurrent('should handle missing code parameter', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
timeout: 5000,
|
||||
})
|
||||
@@ -80,7 +124,7 @@ describe('Function Execute API Route', () => {
|
||||
expect(data).toHaveProperty('error')
|
||||
})
|
||||
|
||||
it('should use default timeout when not provided', async () => {
|
||||
it.concurrent('should use default timeout when not provided', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "test"',
|
||||
useLocalVM: true,
|
||||
@@ -100,7 +144,7 @@ describe('Function Execute API Route', () => {
|
||||
})
|
||||
|
||||
describe('Template Variable Resolution', () => {
|
||||
it('should resolve environment variables with {{var_name}} syntax', async () => {
|
||||
it.concurrent('should resolve environment variables with {{var_name}} syntax', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return {{API_KEY}}',
|
||||
useLocalVM: true,
|
||||
@@ -116,7 +160,7 @@ describe('Function Execute API Route', () => {
|
||||
// The code should be resolved to: return "secret-key-123"
|
||||
})
|
||||
|
||||
it('should resolve tag variables with <tag_name> syntax', async () => {
|
||||
it.concurrent('should resolve tag variables with <tag_name> syntax', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <email>',
|
||||
useLocalVM: true,
|
||||
@@ -132,7 +176,7 @@ describe('Function Execute API Route', () => {
|
||||
// The code should be resolved with the email object
|
||||
})
|
||||
|
||||
it('should NOT treat email addresses as template variables', async () => {
|
||||
it.concurrent('should NOT treat email addresses as template variables', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "Email sent to user"',
|
||||
useLocalVM: true,
|
||||
@@ -151,7 +195,7 @@ describe('Function Execute API Route', () => {
|
||||
// Should not try to replace <waleed@sim.ai> as a template variable
|
||||
})
|
||||
|
||||
it('should only match valid variable names in angle brackets', async () => {
|
||||
it.concurrent('should only match valid variable names in angle brackets', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <validVar> + "<invalid@email.com>" + <another_valid>',
|
||||
useLocalVM: true,
|
||||
@@ -170,64 +214,70 @@ describe('Function Execute API Route', () => {
|
||||
})
|
||||
|
||||
describe('Gmail Email Data Handling', () => {
|
||||
it('should handle Gmail webhook data with email addresses containing angle brackets', async () => {
|
||||
const gmailData = {
|
||||
email: {
|
||||
id: '123',
|
||||
from: 'Waleed Latif <waleed@sim.ai>',
|
||||
to: 'User <user@example.com>',
|
||||
subject: 'Test Email',
|
||||
bodyText: 'Hello world',
|
||||
},
|
||||
rawEmail: {
|
||||
id: '123',
|
||||
payload: {
|
||||
headers: [
|
||||
{ name: 'From', value: 'Waleed Latif <waleed@sim.ai>' },
|
||||
{ name: 'To', value: 'User <user@example.com>' },
|
||||
],
|
||||
it.concurrent(
|
||||
'should handle Gmail webhook data with email addresses containing angle brackets',
|
||||
async () => {
|
||||
const gmailData = {
|
||||
email: {
|
||||
id: '123',
|
||||
from: 'Waleed Latif <waleed@sim.ai>',
|
||||
to: 'User <user@example.com>',
|
||||
subject: 'Test Email',
|
||||
bodyText: 'Hello world',
|
||||
},
|
||||
},
|
||||
rawEmail: {
|
||||
id: '123',
|
||||
payload: {
|
||||
headers: [
|
||||
{ name: 'From', value: 'Waleed Latif <waleed@sim.ai>' },
|
||||
{ name: 'To', value: 'User <user@example.com>' },
|
||||
],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <email>',
|
||||
useLocalVM: true,
|
||||
params: gmailData,
|
||||
})
|
||||
|
||||
const { POST } = await import('@/app/api/function/execute/route')
|
||||
const response = await POST(req)
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
const data = await response.json()
|
||||
expect(data.success).toBe(true)
|
||||
}
|
||||
)
|
||||
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <email>',
|
||||
useLocalVM: true,
|
||||
params: gmailData,
|
||||
})
|
||||
it.concurrent(
|
||||
'should properly serialize complex email objects with special characters',
|
||||
async () => {
|
||||
const complexEmailData = {
|
||||
email: {
|
||||
from: 'Test User <test@example.com>',
|
||||
bodyHtml: '<div>HTML content with "quotes" and \'apostrophes\'</div>',
|
||||
bodyText: 'Text with\nnewlines\tand\ttabs',
|
||||
},
|
||||
}
|
||||
|
||||
const { POST } = await import('@/app/api/function/execute/route')
|
||||
const response = await POST(req)
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <email>',
|
||||
useLocalVM: true,
|
||||
params: complexEmailData,
|
||||
})
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
const data = await response.json()
|
||||
expect(data.success).toBe(true)
|
||||
})
|
||||
const { POST } = await import('@/app/api/function/execute/route')
|
||||
const response = await POST(req)
|
||||
|
||||
it('should properly serialize complex email objects with special characters', async () => {
|
||||
const complexEmailData = {
|
||||
email: {
|
||||
from: 'Test User <test@example.com>',
|
||||
bodyHtml: '<div>HTML content with "quotes" and \'apostrophes\'</div>',
|
||||
bodyText: 'Text with\nnewlines\tand\ttabs',
|
||||
},
|
||||
expect(response.status).toBe(200)
|
||||
}
|
||||
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <email>',
|
||||
useLocalVM: true,
|
||||
params: complexEmailData,
|
||||
})
|
||||
|
||||
const { POST } = await import('@/app/api/function/execute/route')
|
||||
const response = await POST(req)
|
||||
|
||||
expect(response.status).toBe(200)
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
describe('Custom Tools', () => {
|
||||
it('should handle custom tool execution with direct parameter access', async () => {
|
||||
it.concurrent('should handle custom tool execution with direct parameter access', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return location + " weather is sunny"',
|
||||
useLocalVM: true,
|
||||
@@ -246,7 +296,7 @@ describe('Function Execute API Route', () => {
|
||||
})
|
||||
|
||||
describe('Security and Edge Cases', () => {
|
||||
it('should handle malformed JSON in request body', async () => {
|
||||
it.concurrent('should handle malformed JSON in request body', async () => {
|
||||
const req = new NextRequest('http://localhost:3000/api/function/execute', {
|
||||
method: 'POST',
|
||||
body: 'invalid json{',
|
||||
@@ -259,7 +309,7 @@ describe('Function Execute API Route', () => {
|
||||
expect(response.status).toBe(500)
|
||||
})
|
||||
|
||||
it('should handle timeout parameter', async () => {
|
||||
it.concurrent('should handle timeout parameter', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "test"',
|
||||
useLocalVM: true,
|
||||
@@ -277,7 +327,7 @@ describe('Function Execute API Route', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('should handle empty parameters object', async () => {
|
||||
it.concurrent('should handle empty parameters object', async () => {
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return "no params"',
|
||||
useLocalVM: true,
|
||||
@@ -485,7 +535,7 @@ SyntaxError: Invalid or unexpected token
|
||||
expect(data.debug.lineContent).toBe('return a + b + c + d;')
|
||||
})
|
||||
|
||||
it('should provide helpful suggestions for common syntax errors', async () => {
|
||||
it.concurrent('should provide helpful suggestions for common syntax errors', async () => {
|
||||
const mockScript = vi.fn().mockImplementation(() => {
|
||||
const error = new Error('Unexpected end of input')
|
||||
error.name = 'SyntaxError'
|
||||
@@ -517,7 +567,7 @@ SyntaxError: Invalid or unexpected token
|
||||
})
|
||||
|
||||
describe('Utility Functions', () => {
|
||||
it('should properly escape regex special characters', async () => {
|
||||
it.concurrent('should properly escape regex special characters', async () => {
|
||||
// This tests the escapeRegExp function indirectly
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return {{special.chars+*?}}',
|
||||
@@ -534,7 +584,7 @@ SyntaxError: Invalid or unexpected token
|
||||
// Should handle special regex characters in variable names
|
||||
})
|
||||
|
||||
it('should handle JSON serialization edge cases', async () => {
|
||||
it.concurrent('should handle JSON serialization edge cases', async () => {
|
||||
// Test with complex but not circular data first
|
||||
const req = createMockRequest('POST', {
|
||||
code: 'return <complexData>',
|
||||
|
||||
@@ -4,6 +4,7 @@ import { env, isTruthy } from '@/lib/env'
|
||||
import { executeInE2B } from '@/lib/execution/e2b'
|
||||
import { CodeLanguage, DEFAULT_CODE_LANGUAGE, isValidCodeLanguage } from '@/lib/execution/languages'
|
||||
import { createLogger } from '@/lib/logs/console/logger'
|
||||
import { validateProxyUrl } from '@/lib/security/url-validation'
|
||||
import { generateRequestId } from '@/lib/utils'
|
||||
export const dynamic = 'force-dynamic'
|
||||
export const runtime = 'nodejs'
|
||||
@@ -11,6 +12,29 @@ export const maxDuration = 60
|
||||
|
||||
const logger = createLogger('FunctionExecuteAPI')
|
||||
|
||||
function createSecureFetch(requestId: string) {
|
||||
const originalFetch = (globalThis as any).fetch || require('node-fetch').default
|
||||
|
||||
return async function secureFetch(input: any, init?: any) {
|
||||
const url = typeof input === 'string' ? input : input?.url || input
|
||||
|
||||
if (!url || typeof url !== 'string') {
|
||||
throw new Error('Invalid URL provided to fetch')
|
||||
}
|
||||
|
||||
const validation = validateProxyUrl(url)
|
||||
if (!validation.isValid) {
|
||||
logger.warn(`[${requestId}] Blocked fetch request due to SSRF validation`, {
|
||||
url: url.substring(0, 100),
|
||||
error: validation.error,
|
||||
})
|
||||
throw new Error(`Security Error: ${validation.error}`)
|
||||
}
|
||||
|
||||
return originalFetch(input, init)
|
||||
}
|
||||
}
|
||||
|
||||
// Constants for E2B code wrapping line counts
|
||||
const E2B_JS_WRAPPER_LINES = 3 // Lines before user code: ';(async () => {', ' try {', ' const __sim_result = await (async () => {'
|
||||
const E2B_PYTHON_WRAPPER_LINES = 1 // Lines before user code: 'def __sim_main__():'
|
||||
@@ -737,7 +761,7 @@ export async function POST(req: NextRequest) {
|
||||
params: executionParams,
|
||||
environmentVariables: envVars,
|
||||
...contextVariables,
|
||||
fetch: (globalThis as any).fetch || require('node-fetch').default,
|
||||
fetch: createSecureFetch(requestId),
|
||||
console: {
|
||||
log: (...args: any[]) => {
|
||||
const logMessage = `${args
|
||||
|
||||
Reference in New Issue
Block a user