Skip to content

Commit 1da4b15

Browse files
waleedlatif1claude
andcommitted
fix(security): pentest remediation — condition escaping, SSRF hardening, ReDoS protection (#3820)
* fix(executor): escape newline characters in condition expression strings Unescaped newline/carriage-return characters in resolved string values cause unterminated string literals in generated JS, crashing condition evaluation with a SyntaxError. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): prevent ReDoS in guardrails regex validation Add safe-regex2 to reject catastrophic backtracking patterns before execution and cap input length at 10k characters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): SSRF localhost hardening and regex DoS protection Block localhost/loopback URLs in hosted environments using isHosted flag instead of allowHttp. Add safe-regex2 validation and input length limits to regex guardrails to prevent catastrophic backtracking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): validate regex syntax before safety check Move new RegExp() before safe() so invalid patterns get a proper syntax error instead of a misleading "catastrophic backtracking" message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): address PR review feedback - Hoist isLocalhost && isHosted guard to single early-return before protocol checks, removing redundant duplicate block - Move regex syntax validation (new RegExp) before safe-regex2 check so invalid patterns get proper syntax error instead of misleading "catastrophic backtracking" message * fix(security): remove input length cap from regex validation The 10k character cap would block legitimate guardrail checks on long LLM outputs. Input length doesn't affect ReDoS risk — the safe-regex2 pattern check already prevents catastrophic backtracking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(tests): mock isHosted in input-validation and function-execute tests Tests that assert self-hosted localhost behavior need isHosted=false, which is not guaranteed in CI where NEXT_PUBLIC_APP_URL is set to the hosted domain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 296fd89 commit 1da4b15

File tree

8 files changed

+59
-26
lines changed

8 files changed

+59
-26
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ vi.mock('@/lib/execution/e2b', () => ({
2626
executeInE2B: mockExecuteInE2B,
2727
}))
2828

29+
vi.mock('@/lib/core/config/feature-flags', () => ({
30+
isHosted: false,
31+
isE2bEnabled: false,
32+
isProd: false,
33+
isDev: false,
34+
isTest: true,
35+
}))
36+
2937
import { validateProxyUrl } from '@/lib/core/security/input-validation'
3038
import { POST } from '@/app/api/function/execute/route'
3139

apps/sim/executor/variables/resolver.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,13 @@ export class VariableResolver {
236236
}
237237

238238
if (typeof resolved === 'string') {
239-
const escaped = resolved.replace(/\\/g, '\\\\').replace(/'/g, "\\'")
239+
const escaped = resolved
240+
.replace(/\\/g, '\\\\')
241+
.replace(/'/g, "\\'")
242+
.replace(/\n/g, '\\n')
243+
.replace(/\r/g, '\\r')
244+
.replace(/\u2028/g, '\\u2028')
245+
.replace(/\u2029/g, '\\u2029')
240246
return `'${escaped}'`
241247
}
242248
if (typeof resolved === 'object' && resolved !== null) {

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import https from 'https'
44
import type { LookupFunction } from 'net'
55
import { createLogger } from '@sim/logger'
66
import * as ipaddr from 'ipaddr.js'
7+
import { isHosted } from '@/lib/core/config/feature-flags'
78
import { type ValidationResult, validateExternalUrl } from '@/lib/core/security/input-validation'
89

910
const logger = createLogger('InputValidation')
@@ -89,10 +90,7 @@ export async function validateUrlWithDNS(
8990
return ip === '127.0.0.1' || ip === '::1'
9091
})()
9192

92-
if (
93-
isPrivateOrReservedIP(address) &&
94-
!(isLocalhost && resolvedIsLoopback && !options.allowHttp)
95-
) {
93+
if (isPrivateOrReservedIP(address) && !(isLocalhost && resolvedIsLoopback && !isHosted)) {
9694
logger.warn('URL resolves to blocked IP address', {
9795
paramName,
9896
hostname,

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
2323
import { sanitizeForLogging } from '@/lib/core/security/redaction'
2424

2525
vi.mock('@sim/logger', () => loggerMock)
26+
vi.mock('@/lib/core/config/feature-flags', () => ({
27+
isHosted: false,
28+
}))
2629

2730
describe('validatePathSegment', () => {
2831
describe('valid inputs', () => {
@@ -569,25 +572,25 @@ describe('validateUrlWithDNS', () => {
569572
expect(result.error).toContain('https://')
570573
})
571574

572-
it('should accept https localhost URLs', async () => {
575+
it('should accept https localhost URLs (self-hosted)', async () => {
573576
const result = await validateUrlWithDNS('https://localhost/api')
574577
expect(result.isValid).toBe(true)
575578
expect(result.resolvedIP).toBeDefined()
576579
})
577580

578-
it('should accept http localhost URLs', async () => {
581+
it('should accept http localhost URLs (self-hosted)', async () => {
579582
const result = await validateUrlWithDNS('http://localhost/api')
580583
expect(result.isValid).toBe(true)
581584
expect(result.resolvedIP).toBeDefined()
582585
})
583586

584-
it('should accept IPv4 loopback URLs', async () => {
587+
it('should accept IPv4 loopback URLs (self-hosted)', async () => {
585588
const result = await validateUrlWithDNS('http://127.0.0.1/api')
586589
expect(result.isValid).toBe(true)
587590
expect(result.resolvedIP).toBeDefined()
588591
})
589592

590-
it('should accept IPv6 loopback URLs', async () => {
593+
it('should accept IPv6 loopback URLs (self-hosted)', async () => {
591594
const result = await validateUrlWithDNS('http://[::1]/api')
592595
expect(result.isValid).toBe(true)
593596
expect(result.resolvedIP).toBeDefined()
@@ -918,7 +921,7 @@ describe('validateExternalUrl', () => {
918921
})
919922
})
920923

921-
describe('localhost and loopback addresses', () => {
924+
describe('localhost and loopback addresses (self-hosted)', () => {
922925
it.concurrent('should accept https localhost', () => {
923926
const result = validateExternalUrl('https://localhost/api')
924927
expect(result.isValid).toBe(true)
@@ -1027,7 +1030,7 @@ describe('validateImageUrl', () => {
10271030
expect(result.isValid).toBe(true)
10281031
})
10291032

1030-
it.concurrent('should accept localhost URLs', () => {
1033+
it.concurrent('should accept localhost URLs (self-hosted)', () => {
10311034
const result = validateImageUrl('https://localhost/image.png')
10321035
expect(result.isValid).toBe(true)
10331036
})

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import * as ipaddr from 'ipaddr.js'
3+
import { isHosted } from '@/lib/core/config/feature-flags'
34

45
const logger = createLogger('InputValidation')
56

@@ -710,20 +711,21 @@ export function validateExternalUrl(
710711
}
711712
}
712713

714+
if (isLocalhost && isHosted) {
715+
return {
716+
isValid: false,
717+
error: `${paramName} cannot point to localhost`,
718+
}
719+
}
720+
713721
if (options.allowHttp) {
714722
if (protocol !== 'https:' && protocol !== 'http:') {
715723
return {
716724
isValid: false,
717725
error: `${paramName} must use http:// or https:// protocol`,
718726
}
719727
}
720-
if (isLocalhost) {
721-
return {
722-
isValid: false,
723-
error: `${paramName} cannot point to localhost`,
724-
}
725-
}
726-
} else if (protocol !== 'https:' && !(protocol === 'http:' && isLocalhost)) {
728+
} else if (protocol !== 'https:' && !(protocol === 'http:' && isLocalhost && !isHosted)) {
727729
return {
728730
isValid: false,
729731
error: `${paramName} must use https:// protocol`,
Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import safe from 'safe-regex2'
2+
13
/**
24
* Validate if input matches regex pattern
35
*/
@@ -7,15 +9,23 @@ export interface ValidationResult {
79
}
810

911
export function validateRegex(inputStr: string, pattern: string): ValidationResult {
12+
let regex: RegExp
1013
try {
11-
const regex = new RegExp(pattern)
12-
const match = regex.test(inputStr)
13-
14-
if (match) {
15-
return { passed: true }
16-
}
17-
return { passed: false, error: 'Input does not match regex pattern' }
14+
regex = new RegExp(pattern)
1815
} catch (error: any) {
1916
return { passed: false, error: `Invalid regex pattern: ${error.message}` }
2017
}
18+
19+
if (!safe(pattern)) {
20+
return {
21+
passed: false,
22+
error: 'Regex pattern rejected: potentially unsafe (catastrophic backtracking)',
23+
}
24+
}
25+
26+
const match = regex.test(inputStr)
27+
if (match) {
28+
return { passed: true }
29+
}
30+
return { passed: false, error: 'Input does not match regex pattern' }
2131
}

apps/sim/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@
116116
"es-toolkit": "1.45.1",
117117
"ffmpeg-static": "5.3.0",
118118
"fluent-ffmpeg": "2.1.3",
119-
"free-email-domains": "1.2.25",
120119
"framer-motion": "^12.5.0",
120+
"free-email-domains": "1.2.25",
121121
"google-auth-library": "10.5.0",
122122
"gray-matter": "^4.0.3",
123123
"groq-sdk": "^0.15.0",
@@ -174,6 +174,7 @@
174174
"remark-gfm": "4.0.1",
175175
"resend": "^4.1.2",
176176
"rss-parser": "3.13.0",
177+
"safe-regex2": "5.1.0",
177178
"sharp": "0.34.3",
178179
"soap": "1.8.0",
179180
"socket.io": "^4.8.1",

bun.lock

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)