T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:37
- Finding
- Unvalidated Custom Feature Path Reaches a Bash File Operation<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 37–39 and 296–300 **Vulnerability Type**: Command injection and path traversal through unsafe path handling **Risk Level**: High ### Vulnerable Code Selection of an unrestricted custom path: ```markdown 4. 通过 `AskUserQuestion` 展示**最新的 3 个** feature 目录： - 每个选项显示目录名 - 用户可以通过 "Other" 输入自定义路径 - 所有问题文本用中文："选择要归档的 feature：" 5. 读取选中 feature 的 `spec-design.md` ``` Use of the selected feature directory in a Bash operation: ```markdown ## Step 8: 执行归档 1. 如果不存在，创建 `spec/archive/` 目录 2. 移动整个 feature 目录：`spec/{feature_dir}/` → `spec/archive/{feature_dir}/` - 保留原始目录名 - 使用 Bash `mv` 命令 ``` ### Technical Analysis The skill permits the user to enter an arbitrary custom path through the “Other” option. It does not require the resulting path to be canonicalized, confined to `spec/`, or validated as a direct child directory whose name matches the expected `feature_*` pattern. The selected value is subsequently used to construct paths for reading feature files and is ultimately interpolated as `{feature_dir}` into a Bash `mv` operation. The instructions do not require shell-safe argument handling, quoting, use of `--`, or rejection of shell metacharacters. This creates two related attack surfaces: 1. **Path traversal:** Values containing `..`, absolute paths, or separators may escape the intended `spec/feature_*` scope and cause unrelated accessible files to be read or moved. 2. **Command injection:** If an implementing agent inserts the custom value directly into a shell command as instructed, shell metacharacters such as command substitutions, separators, or redirections may be interpreted by Bash. Successful shell injection depends on the exact command generated by the executing agent. However, the skill explicitly combines unrestricted path input with a Bash command and provides no mandatory validation or escaping controls. ### Attack Path 1. An attacker or untrusted user invok ...[truncated 1505 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Eliminate unrestricted path input** - Prefer selection only from directories discovered by the skill. - If custom selection is required, accept only a directory basename rather than an arbitrary path. 2. **Apply strict allowlist validation** - Require the basename to match a conservative pattern such as `^feature_[A-Za-z0-9._-]+$`. - Reject absolute paths, `..`, path separators, null bytes, control characters, newlines, and shell metacharacters. - Reject symbolic links unless they are explicitly required and safely resolved. 3. **Canonicalize and enforce containment** - Resolve both the selected source and intended archive destination to canonical paths. - Verify that the source is a direct child of the canonical `spec/` directory. - Verify that the destination is a direct child of the canonical `spec/archive/` directory. - Abort if either resolved path escapes its approved parent directory. 4. **Avoid shell execution** - Use a structured filesystem move API that accepts source and destination as separate path values. - Do not build a shell command by concatenating or interpolating user-controlled strings. 5. **Harden Bash use if it is unavoidable** - Pass validated paths as separately quoted arguments. - Use `mv -- "$source" "$destination"` so option-like path values cannot be interpreted as flags. - Do not use `eval`, command substitution, or a dynamically assembled command string. 6. **Validate archive state** - Require the source to exist and be a directory. - Require `spec-design.md` to be a regular file inside the validated source directory. - Refuse to overwrite an existing archive destination without explicit confirmation. - Revalidate paths immediately before the move to reduce symbolic-link and time-of-check/time-of-use risks. ]]>