Skip to content

replaceVariables interpolates the variable key into a RegExp unescaped (over-match + crash/ReDoS) #262

@aosmcleod

Description

@aosmcleod

Summary

In replaceVariables (src/shared/config.ts), the variable key is interpolated raw into a RegExp source string, so regex metacharacters in the key are not treated literally. This causes (a) over-matching — the . in keys like user_config.cs_password matches any character — and (b) a hard crash / ReDoS if a manifest's user_config key contains a metacharacter (the schema allows arbitrary string keys).

This is distinct from #258 (which is about $ in the replacement value); this is about the key in the pattern.

Mechanism

src/shared/config.ts:27:

const pattern = new RegExp(`\\$\\{${key}\\}`, "g");

key is user_config.<name> (plus built-ins). The . is an unescaped metacharacter.

Reproduction (executed)

  • Over-match: variables = { "user_config.cs_password": "SECRET_PW" }, template "${user_config_cs_password}" (underscore where the dot is) → "SECRET_PW". Expected: unchanged.
  • Crash: a user_config key named [ yields new RegExp("\\$\\{user_config.[\\}","g")SyntaxError: Invalid regular expression: Unterminated character class, thrown from the string branch that runs for every string in the config, so getMcpConfigForManifest throws for an otherwise valid manifest. A key like (a+)+ is a ReDoS vector.

Suggested fix

Escape the key before building the regex:

const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const pattern = new RegExp(`\\$\\{${escaped}\\}`, "g");

(The replacement-function fix for #258 handles $ in the value; this handles metacharacters in the key.)

Environment: current main (70fe3b3).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions