T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:127- Finding
Sed Program Injection Through Configuration Values
- 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 Both `key` and `val` originate from command-line arguments and are inserted directly into a sed program. They are not escaped for the selected `|` delimiter, regular-expression syntax, replacement syntax, command separators, or control characters. An attacker can place a sed delimiter and additional commands in an argument. GNU sed supports the `e` command, which executes a command using the system shell. Consequently, this is not limited to corrupting configuration data: a malicious value can terminate the intended substitution expression and inject an additional sed command. The key is also interpreted as a regular expression by both `grep` and `sed`. Even when command execution is not achieved, metacharacters can select or overwrite unintended configuration records. ### Attack Path 1. The attacker invokes the Skill in a context where they can supply arguments to `config`. 2. The attacker first creates a normal configuration record if one does not already exist: ```bash scripts/script.sh config safe initial ``` 3. The attacker invokes the update path ...[truncated 1071 chars]- Remediation
View remediation
&2 return 1 } ``` - Reject carriage returns, line feeds, NUL-equivalent input, and other control characters in values. - Replace the dynamic sed update with a parser that compares keys as literal strings and writes to a securely created temporary file. - Create the temporary file with `mktemp` inside a trusted directory, set restrictive permissions, and atomically rename it after a successful write. - If sed must be retained, escape regular-expression, replacement, delimiter, backslash, ampersand, and newline characters separately. Input validation should still be applied. - Add tests covering `|`, `&`, backslashes, newlines, regular-expression metacharacters, and sed command-injection payloads. ]]>