Skip to content

security: [HIGH] Shell expansion in growth.sh safe_substitute() function #3180

@louisgv

Description

@louisgv

Finding

File: .claude/skills/setup-agent-team/growth.sh:42
Severity: HIGH

Description

The safe_substitute() function uses sed -i.bak with runtime-generated delimiters, but the ${escaped} variable is still subject to shell expansion before sed runs. While the function escapes sed metacharacters (backslashes, ampersands), it doesn't prevent shell interpretation of the variable content.

Vulnerable code:

escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g')
sed -i.bak "s$(printf '\x01')${placeholder}$(printf '\x01')${escaped}$(printf '\x01')g" "$file"

If $escaped contains shell metacharacters like $, \``, or $(...)`, they will be expanded by the shell before sed sees them.

Attack Scenario

If Reddit credentials or other substituted values contain shell metacharacters, they could trigger unintended command execution during the substitution step.

Recommendation

Use one of these approaches:

  1. Pass the substitution via a here-string or temp script to avoid shell expansion
  2. Switch to bun -e for file transformations (already used elsewhere in the codebase)
  3. Use printf '%s' "$escaped" | sed ... to avoid interpolation

Example using bun:

bun -e "const fs = require('fs'); const content = fs.readFileSync('$file', 'utf8'); fs.writeFileSync('$file', content.replace('$placeholder', process.env.VALUE));" 

Discovered by shell-scanner

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-human-reviewIssue needs human review before automated processingsafe-to-workSecurity triage: safe for automated processingsecuritySecurity vulnerabilities and concerns

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions