T09 · Insecure Skill Coding Practices
- Location
scripts/script.sh:145- Finding
Arbitrary Command Execution Through Sed Expression Injection
- Content
View full analysis
Vulnerability Details
File Location:
scripts/script.sh, lines 145-148
Vulnerability Type: Shell command injection through an attacker-controlled GNU sed expression
Risk Level: HighVulnerable Code
bash if [ -f "$cfg" ] && grep -q "^${key}=" "$cfg" 2>/dev/null; then sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" else echo "${key}=${val}" >> "$cfg" fiTechnical Analysis
The
configcommand interpolates the user-controlledkeyandvalarguments directly into a sed program. Neither argument is validated nor escaped for use in a sed regular expression and replacement expression.In the update branch, an attacker can include the
|delimiter invalto terminate the replacement expression and append GNU sed'seflag. Theeflag executes the resulting pattern space through/bin/sh.For example, if the existing key is
xand the supplied value is;id|e, the generated command is effectively:bash sed -i 's|^x=.*|x=;id|e' "$cfg"When a matching configuration line is processed, GNU sed executes the resulting text
x=;idas a shell command. The same technique can execute any command available to the account running the Skill.The configuration key is also embedded into the sed regular expression without validation. This permits regular-expression manipulation and can cause unintended configuration entries to be selected, although arbitrary command execution through the replacement and
eflag is the principal security impact.Attack Path
-
The attacker must be able to influence arguments passed to
scripts/script.sh config, such as through an Agent request or another caller that exposes the Skill's documented interface. -
The attacker creates a configuration key so that the vulnerable update branch will be used:
bash scripts/script.sh config x safe -
The attacker updates that key with a value that closes th ...[truncated 1524 chars]
-
- Remediation
View remediation
Remediation Suggestions
Do not construct sed programs by directly interpolating untrusted configuration keys or values.
-
Strictly validate configuration keys. Accept only a limited identifier syntax, for example:
bash if [[ ! "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then echo "Invalid configuration key" >&2 return 1 fi -
Replace the sed-based update with a non-evaluating implementation. Write to a securely created temporary file and pass values as data rather than executable syntax. For example, an implementation may read each line, compare its key using shell string operations, and write either the replacement or original line with
printf '%s\n'. -
Use a secure temporary file in the same directory, set restrictive permissions, and atomically rename it over the original file only after successful generation.
-
Treat values as opaque data. Use
printf '%s=%s\n' "$key" "$val"rather thanecho, and reject or explicitly encode newline characters if the configuration format requires one entry per line. -
Avoid GNU sed's
efunctionality entirely for configuration processing. Merely escaping the delimiter is insufficient because regular-expression metacharacters, replacement metacharacters, backslashes, and newlines would all require context-specific handling. -
Add regression tests using values containing sed delimiters, backslashes, ampersands, newlines, and strings such as
;id|e. Tests should verify that these inputs are either safely stored as data or rejected and that no external command is executed.
-