T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/gh_accel.sh:151
- Finding
- Failed clone operations can recursively delete an existing user-controlled directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/gh_accel.sh`, lines 151-172 **Vulnerability Type**: Unsafe recursive deletion of a user-controlled path **Risk Level**: High ### Vulnerable Code ```bash cmd_clone() { local no_direct=0 repo dir if [ "$1" = "--no-direct" ]; then no_direct=1; repo="$2"; dir="${3:-}" else repo="$1"; dir="${2:-}" fi repo="${repo#https://github.com/}"; repo="${repo%.git}" [ -z "$dir" ] && dir="$(basename "$repo")" if [ $no_direct -eq 0 ]; then echo "→ 直连: https://github.com/$repo" if git clone "https://github.com/$repo" "$dir"; then echo "✅ 直连 clone 成功 -> $dir" return 0 fi rm -rf "$dir" echo " ✗ 直连失败，降级镜像…" fi for base in "${CLONE_PROXIES[@]}"; do echo "→ 镜像: $base/$repo" if git clone "$base/$repo" "$dir"; then echo "✅ 经镜像 clone 成功 -> $dir" echo "⚠️ remote 指向镜像，push 前执行:" echo " git -C $dir remote set-url origin https://github.com/$repo.git" return 0 fi rm -rf "$dir" echo " ✗ 失败" done ``` ### Technical Analysis The clone destination is taken directly from a command-line argument and passed to `rm -rf` whenever a clone attempt fails. The script does not verify that the destination was created by the current invocation. A normal `git clone` operation fails when its destination already exists and is not empty. Consequently, supplying an existing directory causes the clone to fail and immediately triggers recursive deletion of that directory. The same unsafe cleanup occurs after each failed mirror attempt. There are no canonical-path checks, protected-path checks, ownership checks, or state flags recording whether the script created the destination. The `rm` invocation also lacks the `--` option terminator, which is an additional unsafe path-handling practice for option-like names. ### Attack Path 1. An attacker, unsafe automation workflow, or mistaken user supplies a valuable existing directory as the dest ...[truncated 1000 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Refuse to operate when the requested destination already exists: ```bash if [ -e "$dir" ] || [ -L "$dir" ]; then printf 'Error: destination already exists: %s\n' "$dir" >&2 return 1 fi ``` 2. Create a private temporary directory and clone into it. Rename it to the requested destination only after a successful clone. 3. Track explicitly whether the current invocation created a path. Cleanup must only remove a path proven to have been created by that invocation. 4. Canonicalize and validate the destination before use. Reject empty paths, root directories, home directories, parent traversal, protected locations, and option-like path values. 5. Use an option terminator for deletion: ```bash rm -rf -- "$temporary_directory" ``` 6. Install an `EXIT` trap that cleans only the private temporary directory, rather than deleting the final user-selected destination after clone failures. 7. Add regression tests covering pre-existing non-empty directories, symlinks, relative traversal paths, option-like names, and failed direct and mirror clone attempts. ]]>