T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:132- Finding
Arbitrary Command Execution Through Sed Program Injection
- Content
View full analysis
)"; fi elif [ -z "$val" ]; then grep "^${key}=" "$cfg" 2>/dev/null | cut -d= -f2- || echo "(not set)" else if [ -f "$cfg" ] && grep -q "^${key}=" "$cfg" 2>/dev/null; then sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" else echo "${key}=${val}" >> "$cfg" fi echo "Set: $key=$val" fi } ``` ### Technical Analysis The `key` and `val` arguments are interpolated directly into a sed program: ```bash sed -i "s|^${key}=.*|${key}=${val}|" "$cfg" ``` Shell quoting prevents a direct shell metacharacter injection at the initial command-parsing stage, but it does not make the resulting sed program safe. An attacker can supply sed delimiters, newlines, regular-expression operators, replacement operators, or additional sed commands. On GNU sed, the `e` command and the `e` substitution flag execute commands through a shell. A value containing embedded newlines can terminate the intended substitution and introduce a separate `e` command. The final delimiter generated by the script can be neutralized as a sed comment. Exploitation requires the targeted configuration key to exist so that execution reaches the `sed -i` branch. The `key` is also interpreted as a regular expression in both `grep` and `sed`, allowing an attacker to select or overwrite unintended configuration records. ### Attack Path 1. The attacker causes the skill or an Agent to invoke `config` with attacker-controlled a ...[truncated 1465 chars]- Remediation
View remediation
&2 return 1 } ``` - Reject control characters and newline characters in configuration values if multiline values are not explicitly required. - Replace the sed update with a safely parameterized implementation using a language that performs exact string comparisons rather than regular-expression evaluation. - Write updates to a securely created temporary file in the same directory and atomically rename it over the original file. - Apply restrictive permissions to the temporary and final configuration files. - If sed must be retained, escape regular-expression and replacement syntax separately; however, a redesign that avoids executable sed source generation is strongly preferred. - Add regression tests covering delimiters, backslashes, ampersands, newlines, regex metacharacters, and GNU sed commands such as `e`. ]]>