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 abfa2f8.

* lint

* ack PR comments

Author: waleedlatif1

apps/sim/app/api/files/utils.test.ts

@@ -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()
+      })
+    })
+  })
+})

apps/sim/app/api/files/utils.ts

@@ -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
+ */
+function sanitizeFilename(filename: string): string {
+  if (!filename || typeof filename !== 'string') {
+    throw new Error('Invalid filename provided')
+  }
+
+  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 {
-  const possiblePaths = [join(UPLOAD_DIR, filename), join(process.cwd(), 'uploads', filename)]
+  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')]
 
-  for (const path of possiblePaths) {
-    if (existsSync(path)) {
-      return path
+      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
+    return null
+  } catch (error) {
+    logger.error('Error in findLocalFile:', error)
+    return null
+  }
 }
 
 const SAFE_INLINE_TYPES = new Set([