T09 · Insecure Skill Coding Practices
Error
- Location
SKILL.md:230- Finding
Arbitrary Command Execution Through Unsafe Generated Shell Environment File
- Content
View full analysis
"$_STATE_DIR/env.sh" <" ``` ### Technical Analysis The Skill serializes multiple dynamically derived values into an executable shell script without shell escaping. These values include the repository path, Git branch name, state paths, and potentially a user-provided test command. Placing a value between literal double quotes in generated shell source is not safe unless shell metacharacters are escaped. A value containing a double quote can terminate the assignment, while characters such as semicolons, command substitutions, variable expansions, and comment markers can introduce executable shell syntax. Although interpolation into the heredoc does not immediately reinterpret shell syntax contained in variable values, the malicious syntax is written verbatim into `env.sh`. The later `source` operation parses that file as shell code, activating the injected commands. Git reference names permit several shell metacharacters, including double quotes, dollar signs, parentheses, semicolons, and comment markers in otherwise valid names. Consequently, a malicious repository can use a crafted branch name to inje ...[truncated 2279 chars]- Remediation
View remediation
"$_STATE_DIR/env.json" ``` Read individual fields as data rather than sourcing the file: ```bash _REPO_ROOT=$(jq -r '.repo_root' "$_STATE_DIR/env.json") _BRANCH=$(jq -r '.branch' "$_STATE_DIR/env.json") ``` If an executable environment file is unavoidable, serialize every value with shell-safe quoting: ```bash { printf 'export _REPO_ROOT=%q\n' "$_REPO_ROOT" printf 'export _BASE=%q\n' "$_BASE" printf 'export _BASE_SHA=%q\n' "$_BASE_SHA" printf 'export _BRANCH=%q\n' "$_BRANCH" printf 'export _BRANCH_SAFE=%q\n' "$_BRANCH_SAFE" printf 'export _STATE_DIR=%q\n' "$_STATE_DIR" printf 'export SLUG=%q\n' "$SLUG" printf 'export STACK=%q\n' "$STACK" printf 'export TEST_CMD=%q\n' "$TEST_CMD" printf 'export LINT_CMD=%q\n' "$LINT_CMD" printf 'export _COMMIT_FORMAT=%q\n' "$_COMMIT_FORMAT" printf 'export _TEL=%q\n' "$_TEL" printf 'export _TEL_START=%q\n' "$_TEL_START" printf 'export _SESSION_ID=%q\n' "$_SESSION_ID" printf 'export _OUTCOME=%q\n' "$_OUTCOME" } > "$_STATE_DIR/env.sh" chmod 600 "$_STATE_DIR/env.sh" ``` Additional hardening should include: 1. Validate Git branch names and reject unexpected shell metacharacters before generating state. 2. Keep user-provided test and lint commands as argument arrays or explicitly approved commands rather than executable strings. 3. Avoid executing `$TEST_CMD` and `$LINT_CMD` through implicit shell evaluation where possible. 4. Create state files with restrictive permissions and ensure the state directory is owned by the current u ...[truncated 284 chars]