Skip to content

Commit deb218c

Browse files
P A M O DMatteo
andauthored
Fix #788: Fix quadratic ReDoS in cURL/Postman import placeholder regexes (#789)
* Fix #788: Fix quadratic ReDoS in cURL/Postman import placeholder regexes * Fix #788: address review feedback - use linear regex shape everywhere - Replace all {{\s*([^{}\s]+)\s*}} with {{([^{}]+)\}} for linear-time matching - Add .trim() where variable names are captured - Revert package-lock.json change - Add ReDoS regression tests for env-interpolation, curl.parser, postman.parser * Review follow-up: keep package-lock as on main, CI-safe timing bounds, last loose placeholder regex - package-lock.json back to main's version (the libc removals came from a different npm, not from the fix). - The ReDoS specs allow 250 ms instead of 100: the fixed patterns take ~1 ms, the old ones take seconds at 50k chars, so the test still catches a regression without flaking on a slow CI runner. - adapters.service.ts hasUsableValue() had the same /\{\{[^}]+\}\}/ shape: 3.2 s on 50k '{' against 1 ms now. --------- Co-authored-by: Matteo <keysersoft@gmail.com>
1 parent 83fe503 commit deb218c

10 files changed

Lines changed: 123 additions & 28 deletions

‎packages/backend/src/adapters/adapters.service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,8 @@ export type ImportProbeResult =
465465

466466
/** Set, and not a `{{VAR}}` placeholder left over from the template. */
467467
function hasUsableValue(value: unknown): boolean {
468-
return typeof value === 'string' && value.trim() !== '' && !/\{\{[^}]+\}\}/.test(value);
468+
// [^{}] rather than [^}]: linear on a run of '{' (#788).
469+
return typeof value === 'string' && value.trim() !== '' && !/\{\{[^{}]+\}\}/.test(value);
469470
}
470471

471472
/** A short, printable slice of the probe's response for the install form. */

‎packages/backend/src/common/env-interpolation.util.spec.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,14 @@ describe('EnvInterpolation', () => {
3333
});
3434

3535
it('should return a non-string template unchanged (no throw)', () => {
36-
// Static tools omit `path`; interpolating undefined must not crash.
3736
expect(interpolateString(undefined as unknown as string, envVars))
3837
.toBeUndefined();
3938
});
39+
40+
it('should handle {{ VAR }} with spaces', () => {
41+
expect(interpolateString('{{ API_BASE }}/users', envVars))
42+
.toBe('https://api.example.com/users');
43+
});
4044
});
4145

4246
describe('interpolateDeep', () => {
@@ -91,9 +95,6 @@ describe('EnvInterpolation', () => {
9195
});
9296

9397
it('should not throw for a static tool with no path', () => {
94-
// Regression: a `static` tool endpointMapping has no `path`; with a
95-
// connector that HAS env vars, interpolation used to crash on
96-
// interpolateString(undefined).
9798
const config = { baseUrl: 'https://v3.football.api-sports.io' };
9899
const mapping = {
99100
method: 'static',
@@ -104,4 +105,22 @@ describe('EnvInterpolation', () => {
104105
expect(result.config.baseUrl).toBe('https://v3.football.api-sports.io');
105106
});
106107
});
108+
109+
// ── ReDoS regression ──────────────────────────────────────────────────────
110+
111+
describe('ReDoS regression', () => {
112+
it('should handle a long brace run in under 100ms', () => {
113+
const hostile = 'https://api.example.com/' + '{{'.repeat(50000);
114+
const start = Date.now();
115+
interpolateString(hostile, envVars);
116+
expect(Date.now() - start).toBeLessThan(250);
117+
});
118+
119+
it('should handle {{ followed by a long whitespace run in under 100ms', () => {
120+
const hostile = 'https://api.example.com/{{' + ' '.repeat(50000);
121+
const start = Date.now();
122+
interpolateString(hostile, envVars);
123+
expect(Date.now() - start).toBeLessThan(250);
124+
});
125+
});
107126
});

‎packages/backend/src/common/env-interpolation.util.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* interpolate('{{BASE_URL}}/v1/users', envVars) → 'https://api.example.com/v1/users'
1111
*/
1212

13-
const VAR_PATTERN = /\{\{([^}]+)\}\}/g;
13+
const VAR_PATTERN = /\{\{([^{}]+)\}\}/g;
1414

1515
export interface InterpolateOptions {
1616
/**

‎packages/backend/src/common/unresolved-placeholders.util.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* the caller's business, not a missing credential.
1818
*/
1919

20-
const VAR_PATTERN = /\{\{([^}]+)\}\}/g;
20+
const VAR_PATTERN = /\{\{([^{}]+)\}\}/g;
2121

2222
/** Every `{{VAR}}` name still present anywhere in the value, deduplicated. */
2323
export function findUnresolvedPlaceholders(value: unknown): string[] {

‎packages/backend/src/connectors/catalog-env-rebuild.util.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { CALLER_CONTEXT_PREFIX } from '../common/caller-context.util';
2929
* name when the new one is not set.
3030
*/
3131

32-
const VAR_PATTERN = /\{\{([^}]+)\}\}/g;
32+
const VAR_PATTERN = /\{\{([^{}]+)\}\}/g;
3333

3434
export interface CatalogTemplate {
3535
connector: {

‎packages/backend/src/connectors/connector-secrets.util.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export function isSecretValue(value: string): boolean {
107107
*/
108108
const SECRET_SLOT = /secret|passw|token|key|credential|private|signature|assertion|authorization|cookie|session|jwt|bearer/i;
109109
const NOT_A_SLOT = /(url|uri|endpoint|path)$/i;
110-
const VAR_PATTERN = /\{\{([^}]+)\}\}/g;
110+
const VAR_PATTERN = /\{\{([^{}]+)\}\}/g;
111111

112112
function collectSlotVars(node: unknown, key: string, out: Set<string>): void {
113113
if (typeof node === 'string') {

‎packages/backend/src/connectors/parsers/curl.parser.spec.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,4 +431,29 @@ curl -X DELETE https://api.example.com/users/1`;
431431
expect(params.required).toContain('org_id');
432432
expect(params.required).toContain('project_id');
433433
});
434+
435+
// ── ReDoS regression ──────────────────────────────────────────────────────
436+
437+
describe('ReDoS regression', () => {
438+
it('should handle a long brace run in the URL in under 100ms', () => {
439+
const hostile = `curl https://api.example.com/${'{{'.repeat(50000)}`;
440+
const start = Date.now();
441+
parser.parse(hostile);
442+
expect(Date.now() - start).toBeLessThan(250);
443+
});
444+
445+
it('should handle {{ followed by a long whitespace run in under 100ms', () => {
446+
const hostile = `curl https://api.example.com/{{${' '.repeat(50000)}`;
447+
const start = Date.now();
448+
parser.parse(hostile);
449+
expect(Date.now() - start).toBeLessThan(250);
450+
});
451+
452+
it('should handle a long brace run in the body in under 100ms', () => {
453+
const hostile = `curl -X POST https://api.example.com/ -d '${'{{'.repeat(50000)}'`;
454+
const start = Date.now();
455+
parser.parse(hostile);
456+
expect(Date.now() - start).toBeLessThan(250);
457+
});
458+
});
434459
});

‎packages/backend/src/connectors/parsers/curl.parser.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ export class CurlParser {
105105
const headerMapping: Record<string, string> = {};
106106

107107
// Extract variables from URL path
108-
const pathVars = path.match(/\{\{([^}]+)\}\}/g) || [];
108+
const pathVars = path.match(/\{\{([^{}]+)\}\}/g) || [];
109109
for (const match of pathVars) {
110-
const varName = match.replace(/\{\{|\}\}/g, '');
110+
const varName = match.replace(/\{\{|\}\}/g, '').trim();
111111
properties[varName] = { type: 'string', description: `Path variable: ${varName}` };
112112
required.push(varName);
113113
}
@@ -138,7 +138,7 @@ export class CurlParser {
138138
if (lowerKey === 'authorization') continue; // Handle separately
139139

140140
if (value.includes('{{')) {
141-
const varName = value.replace(/.*\{\{([^}]+)\}\}.*/, '$1');
141+
const varName = value.match(/\{\{([^{}]+)\}\}/)?.[1]?.trim() ?? value;
142142
properties[varName] = { type: 'string', description: `Header value for ${key}` };
143143
headerMapping[key] = `$${varName}`;
144144
} else {
@@ -152,8 +152,8 @@ export class CurlParser {
152152
// Try JSON parse — replace {{var}} placeholders with sentinel values.
153153
// Handle "{{var}}" (quoted) first to avoid producing ""__var_var__"".
154154
const cleanBody = dataBody
155-
.replace(/"\{\{([^}]+)\}\}"/g, '"__var_$1__"') // "{{var}}" → "__var_var__"
156-
.replace(/\{\{([^}]+)\}\}/g, '"__var_$1__"'); // remaining bare {{var}}
155+
.replace(/"\{\{([^{}]+)\}\}"/g, '"__var_$1__"') // "{{var}}" → "__var_var__"
156+
.replace(/\{\{([^{}]+)\}\}/g, '"__var_$1__"'); // remaining bare {{var}}
157157
const parsed = JSON.parse(cleanBody);
158158

159159
// If parsed result is not an object (e.g. bare "{{var}}" parses as string), treat as raw body
@@ -180,7 +180,7 @@ export class CurlParser {
180180
} catch {
181181
// Not JSON — treat as raw body parameter
182182
if (dataBody.includes('{{')) {
183-
const varMatches = [...dataBody.matchAll(/\{\{([^}]+)\}\}/g)];
183+
const varMatches = [...dataBody.matchAll(/\{\{([^{}]+)\}\}/g)];
184184
if (varMatches.length === 1) {
185185
const varName = varMatches[0][1];
186186
properties[varName] = { type: 'string', description: 'Request body' };
@@ -222,7 +222,7 @@ export class CurlParser {
222222
}
223223

224224
// Normalize path: replace {{var}} with {var}
225-
const normalizedPath = path.replace(/\{\{([^}]+)\}\}/g, '{$1}');
225+
const normalizedPath = path.replace(/\{\{([^{}]+)\}\}/g, '{$1}');
226226

227227
const endpointMapping: ParsedTool['endpointMapping'] = {
228228
method,
@@ -277,7 +277,7 @@ export class CurlParser {
277277
const queryParams: Record<string, string> = {};
278278

279279
// Handle {{variable}} in URL by temporary replacement
280-
const safeUrl = url.replace(/\{\{([^}]+)\}\}/g, 'PLACEHOLDER_$1');
280+
const safeUrl = url.replace(/\{\{([^{}]+)\}\}/g, 'PLACEHOLDER_$1');
281281

282282
try {
283283
const parsed = new URL(safeUrl);
@@ -317,8 +317,8 @@ export class CurlParser {
317317

318318
private generateToolName(method: string, path: string): string {
319319
const cleanPath = path
320-
.replace(/\{[^}]+\}/g, '')
321-
.replace(/\{\{[^}]+\}\}/g, '')
320+
.replace(/\{\{[^{}]+\}\}/g, '')
321+
.replace(/\{[^{}]+\}/g, '')
322322
.replace(/[^a-zA-Z0-9]/g, '_')
323323
.replace(/_+/g, '_')
324324
.replace(/^_|_$/g, '');

‎packages/backend/src/connectors/parsers/postman.parser.spec.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,4 +649,54 @@ describe('PostmanParser', () => {
649649
expect(tools[0].outputSchema).toBeUndefined();
650650
});
651651

652+
// ── ReDoS regression ──────────────────────────────────────────────────────
653+
654+
describe('ReDoS regression', () => {
655+
it('should handle a long brace run in the URL in under 100ms', async () => {
656+
const hostile = 'https://api.example.com/' + '{{'.repeat(50000);
657+
const collection = {
658+
info: { name: 'c', schema: 'https://schema.getpostman.com/json/collection/v2.1.0/collection.json' },
659+
item: [
660+
{ name: 'X', request: { method: 'GET', url: { raw: hostile, path: ['x'] } } },
661+
],
662+
};
663+
const start = Date.now();
664+
await parser.parse(collection);
665+
expect(Date.now() - start).toBeLessThan(250);
666+
});
667+
668+
it('should handle {{ followed by a long whitespace run in under 100ms', async () => {
669+
const hostile = 'https://api.example.com/{{' + ' '.repeat(50000);
670+
const collection = {
671+
info: { name: 'c', schema: 'https://schema.getpostman.com/json/collection/v2.1.0/collection.json' },
672+
item: [
673+
{ name: 'X', request: { method: 'GET', url: { raw: hostile, path: ['x'] } } },
674+
],
675+
};
676+
const start = Date.now();
677+
await parser.parse(collection);
678+
expect(Date.now() - start).toBeLessThan(250);
679+
});
680+
681+
it('should handle a long brace run in the body in under 100ms', async () => {
682+
const hostile = '{{'.repeat(50000);
683+
const collection = {
684+
info: { name: 'c', schema: 'https://schema.getpostman.com/json/collection/v2.1.0/collection.json' },
685+
item: [
686+
{
687+
name: 'X',
688+
request: {
689+
method: 'POST',
690+
url: { raw: 'https://api.example.com/x', path: ['x'] },
691+
body: { mode: 'raw', raw: hostile },
692+
},
693+
},
694+
],
695+
};
696+
const start = Date.now();
697+
await parser.parse(collection);
698+
expect(Date.now() - start).toBeLessThan(250);
699+
});
700+
});
701+
652702
});

‎packages/backend/src/connectors/parsers/postman.parser.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ export class PostmanParser {
167167
const headerMapping: Record<string, string> = {};
168168

169169
// Path parameters (from {{param}} in URL path)
170-
const pathVarMatches = url.path.match(/\{\{([^}]+)\}\}/g) || [];
170+
const pathVarMatches = url.path.match(/\{\{([^{}]+)\}\}/g) || [];
171171
for (const match of pathVarMatches) {
172-
const varName = match.replace(/\{\{|\}\}/g, '');
172+
const varName = match.replace(/\{\{|\}\}/g, '').trim();
173173
if (!variables[varName]) {
174174
// It's a dynamic path parameter, not a static env var
175175
properties[varName] = { type: 'string', description: `Path variable: ${varName}` };
@@ -178,8 +178,8 @@ export class PostmanParser {
178178
}
179179

180180
// Also detect {param} style path params (but not {{param}} which are handled above)
181-
const pathWithoutDoubles = url.path.replace(/\{\{[^}]+\}\}/g, '');
182-
const pathParamMatches = pathWithoutDoubles.match(/\{([^}]+)\}/g) || [];
181+
const pathWithoutDoubles = url.path.replace(/\{\{[^{}]+\}\}/g, '');
182+
const pathParamMatches = pathWithoutDoubles.match(/\{([^{}]+)\}/g) || [];
183183
for (const match of pathParamMatches) {
184184
const varName = match.replace(/[{}]/g, '');
185185
properties[varName] = { type: 'string', description: `Path parameter: ${varName}` };
@@ -239,7 +239,7 @@ export class PostmanParser {
239239
}
240240

241241
// Normalize path: replace {{var}} with {var} for engine interpolation
242-
const normalizedPath = url.path.replace(/\{\{([^}]+)\}\}/g, '{$1}');
242+
const normalizedPath = url.path.replace(/\{\{([^{}]+)\}\}/g, '{$1}');
243243

244244
const endpointMapping: ParsedTool['endpointMapping'] = {
245245
method,
@@ -287,7 +287,7 @@ export class PostmanParser {
287287

288288
if (typeof url === 'string') {
289289
try {
290-
const parsed = new URL(url.replace(/\{\{[^}]+\}\}/g, 'placeholder'));
290+
const parsed = new URL(url.replace(/\{\{[^{}]+\}\}/g, 'placeholder'));
291291
return {
292292
raw: url,
293293
path: url.replace(/^https?:\/\/[^/]+/, ''),
@@ -322,8 +322,8 @@ export class PostmanParser {
322322
// Replace {{var}} with sentinel values for JSON parsing.
323323
// Handle "{{var}}" (quoted) first to avoid producing ""var_placeholder"".
324324
const cleanBody = body.raw
325-
.replace(/"\{\{([^}]+)\}\}"/g, '"__var_$1__"') // "{{var}}" → "__var_var__"
326-
.replace(/\{\{([^}]+)\}\}/g, '"__var_$1__"'); // remaining bare {{var}}
325+
.replace(/"\{\{([^{}]+)\}\}"/g, '"__var_$1__"') // "{{var}}" → "__var_var__"
326+
.replace(/\{\{([^{}]+)\}\}/g, '"__var_$1__"'); // remaining bare {{var}}
327327
const parsed = JSON.parse(cleanBody);
328328

329329
// If parsed result is not an object, treat as raw body
@@ -405,7 +405,7 @@ export class PostmanParser {
405405

406406
// Fallback: method + path
407407
const cleanPath = path
408-
.replace(/\{[^}]+\}/g, '')
408+
.replace(/\{[^{}]+\}/g, '')
409409
.replace(/[^a-zA-Z0-9]/g, '_')
410410
.replace(/_+/g, '_')
411411
.replace(/^_|_$/g, '');

0 commit comments

Comments
 (0)