fix(agents): prevent ReDoS in interpreter heuristic regexes#61903
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d348d5fd3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -418,11 +418,11 @@ function analyzeInterpreterHeuristicsFromUnquoted(raw: string): { | |||
| hasScriptHint: boolean; | |||
| } { | |||
| const hasPython = | |||
| /(?:^|\s|(?<!\\)[|&;()])(?:[A-Za-z_][A-Za-z0-9_]*=.*\s+)*python(?:3(?:\.\d+)?)?(?=$|[\s|&;()<>\n\r`$])/i.test( | |||
| /(?:^|\s|(?<!\\)[|&;()])(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)*python(?:3(?:\.\d+)?)?(?=$|[\s|&;()<>\n\r`$])/i.test( | |||
There was a problem hiding this comment.
Match escaped-space env assignments in interpreter detection
Changing the assignment prefix from =.*\s+ to =\S*\s+ drops valid shell forms where the value contains escaped whitespace (for example FOO=a\ b python bad.py && ...). In those cases analyzeInterpreterHeuristicsFromUnquoted no longer sets hasPython, so hasShellWrappedInterpreterInvocation becomes false and the fail-closed preflight path can be skipped for complex bash -c/wrapper commands that previously would have been blocked and validated.
Useful? React with 👍 / 👎.
…prevent ReDoS The inner `.*\s+` in `(?:[A-Za-z_][A-Za-z0-9_]*=.*\s+)*` creates catastrophic backtracking because both `.*` and `\s+` can match whitespace. When the exec tool processes commands with `VAR=value` assignments followed by whitespace-heavy text (e.g. HTML heredocs), the regex engine hangs permanently at 100% CPU. Replace `.*` with `\S*` in all three instances. Shell prefix variable assignments cannot contain unquoted whitespace in the value, so `\S*` is semantically correct and eliminates the ambiguity. Fixes openclaw#61881
1d348d5 to
71c426d
Compare
|
Landed on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71c426ddc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasInterpreterInvocationInSegment = (rawSegment: string): boolean => { | ||
| const segment = extractUnquotedShellText(rawSegment) ?? rawSegment; | ||
| return /^\s*(?:(?:if|then|do|elif|else|while|until|time)\s+)?(?:[A-Za-z_][A-Za-z0-9_]*=.*\s+)*(?:python(?:3(?:\.\d+)?)?|node)(?=$|[\s|&;()<>\n\r`$])/i.test( | ||
| return /^\s*(?:(?:if|then|do|elif|else|while|until|time)\s+)?(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)*(?:python(?:3(?:\.\d+)?)?|node)(?=$|[\s|&;()<>\n\r`$])/i.test( |
There was a problem hiding this comment.
Accept escaped-space assignments in segment interpreter detection
Using =\S*\s+ in hasInterpreterInvocationInSegment no longer matches valid shell prefixes like then FOO=a\ b python bad.py, because the escaped space splits the value for this regex even though it is still one assignment token to the shell. In complex forms that rely on this predicate (for example if ...; then FOO=a\ b python bad.py; fi), hasInterpreterSegmentScriptHint can still be true while hasInterpreterInvocation becomes false, so the fail-closed preflight branch is skipped and script files run without the intended injection check.
Useful? React with 👍 / 👎.
analyzeInterpreterHeuristicsFromUnquotedandhasInterpreterInvocationInSegmentuse(?:[A-Za-z_][A-Za-z0-9_]*=.*\s+)*to detect shell variable assignments beforepython/nodeinvocations. The inner.*\s+creates catastrophic backtracking — both.*and\s+match whitespace, so the()*quantifier explores exponentially many partitions.When the exec tool processes a command containing
VAR=valuefollowed by whitespace-heavy text (e.g. HTML heredocs), the regex engine hangs permanently at 100% CPU. The gateway becomes unresponsive and requires SIGKILL.Fix: replace
.*with\S*in all three regex instances. Shell prefix variable assignments (VAR=value cmd) cannot contain unquoted whitespace in the value — whitespace terminates the assignment.\S*is semantically correct and eliminates the ambiguity.Verified: 2808-char input hangs forever with
.*, completes in 0ms with\S*. All normal patterns (VAR=value python,A=1 B=2 node, barepython) still match correctly.Fixes #61881