T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/reviewer/review-check.sh:14- Finding
Unvalidated Project Name Allows Directory Traversal During Skill Inspection
- Content
View full analysis
/dev/null; then version=$(grep "^version:" "$skill_dir/SKILL.md" | head -1 | cut -d':' -f2 | tr -d ' ') fi echo "- 版本：$version" # 更新时间 if [ -f "$skill_dir/SKILL.md" ]; then local mtime=$(stat -c %y "$skill_dir/SKILL.md" 2>/dev/null | cut -d' ' -f1) echo "- SKILL.md更新：$mtime" fi # check.sh存在 local check_sh=$(find "$skill_dir/scripts" -name "*check.sh" 2>/dev/null | wc -l) if [ $check_sh -gt 0 ]; then echo "- ✅ check.sh存在" else echo "- ❌ 缺少check.sh" fi # 语法检查 for sh in $(find "$skill_dir/scripts" -name "*.sh" 2>/dev/null); do bash -n "$sh" 2>&1 && echo " ✅ $sh 语法OK" || echo " ❌ $sh 语法错误" done echo "" } main() { local project=${1:-all} # ... if [ "$project" = "all" ]; then for skill in "$SKILL_DIR"/apollo-*; do local name=$(basename "$skill") check_skill "$name" done else check_skill "$project" fi } ``` ### Technical Analysis The script accepts its first command-line argument as `project` and passes it directly to `check_skill`. That function constructs a filesystem path by concatenating the trusted base directory with the unvalidated value: ```bash local skill_dir="$SKILL_DIR/$skill_name" ``` The script does not restrict `skill_name` to a single directory component, canonicalize the resulting path, or verify that the resolved path remain ...[truncated 2585 chars]- Remediation
View remediation
&2 return 1 fi ``` 2. Canonicalize both the base directory and candidate path, then enforce containment: ```bash local base_dir candidate base_dir=$(realpath -- "$SKILL_DIR") || return 1 candidate=$(realpath -- "$base_dir/$skill_name") || { printf 'Error: skill directory does not exist\n' >&2 return 1 } case "$candidate" in "$base_dir"/*) skill_dir=$candidate ;; *) printf 'Error: skill path escapes the skills directory\n' >&2 return 1 ;; esac ``` 3. If symbolic links must not escape the skill root, retain the canonical containment check rather than relying only on a regular expression. 4. Use arrays or null-delimited `find` output when iterating through shell scripts so spaces and special characters in filenames are handled safely: ```bash while IFS= read -r -d '' sh; do if bash -n "$sh" 2>&1; then printf ' ✅ %s syntax OK\n' "$sh" else printf ' ❌ %s syntax error\n' "$sh" fi done < <(find "$skill_dir/scripts" -type f -name '*.sh' -print0 2>/dev/null) ``` 5. Add regression tests covering `../`, absolute paths, symbolic-link escapes, nonexistent targets, whitespace, and valid skill names. ]]>