T09 · Insecure Skill Coding Practices
Warning
- Location
- install.sh:63
- Finding
- Unchecked Installation Destination Allows Recursive Self-Copy and Disk Exhaustion## Vulnerability Details **File Location**: `install.sh:63-68` **Vulnerability Type**: Unvalidated filesystem destination and recursive self-copy **Risk Level**: Medium ### Vulnerable Code ```bash install_to() { mkdir -p "$1/$SKILL_NAME" cp -r "$SOURCE_DIR/." "$1/$SKILL_NAME/" rm -rf "$1/$SKILL_NAME/.git" echo "installed -> $1/$SKILL_NAME" } ``` The destination passed to `install_to` can originate directly from the user-controlled `--dir` argument: ```bash if [ -n "$dir" ]; then install_to "$dir"; echo "完成。"; return; fi ``` ### Technical Analysis The shell installer does not canonicalize or validate the requested destination before recursively copying the entire source directory. If `--dir` resolves to the source directory or one of its descendants, the resulting target directory is created inside the directory being copied. For example, when the installer runs from the project root and receives `--dir .`, the target becomes: ```text <SOURCE_DIR>/yotta-humanize ``` The command below then recursively copies `SOURCE_DIR` into that descendant: ```bash cp -r "$SOURCE_DIR/." "$SOURCE_DIR/yotta-humanize/" ``` As the copy traverses the source, it can encounter its own output and repeatedly reproduce nested `yotta-humanize` directories. This can consume disk space, generate deeply nested paths, leave a partial installation, and cause the installation process or host filesystem to fail. The JavaScript installer contains an outside-source validation at `bin/install.js:120-125`, but no equivalent protection exists in `install.sh`. Existing destination symlinks are also not rejected, so filesystem writes and the subsequent `.git` removal may resolve to locations different from those implied by the textual path. The operation does not grant elevated operating-system privileges. Exploitation is limited to the permissions of the account running the installer. ### Attack Path 1. The victim obtains or clones the project and runs `install.sh`. 2. An atta ...[truncated 1331 chars]
- Remediation
- ## Remediation Suggestions 1. Canonicalize both the source and intended target before creating or copying files: ```bash source_real="$(cd "$SOURCE_DIR" && pwd -P)" parent_real="$(mkdir -p "$1" && cd "$1" && pwd -P)" target_real="$parent_real/$SKILL_NAME" ``` 2. Reject targets equal to or contained within the source directory: ```bash case "$target_real/" in "$source_real/"*) echo "Target directory must be outside the skill source directory." >&2 exit 2 ;; esac ``` 3. Reject a target or relevant parent component if it is a symbolic link. Where available, use `realpath` and verify the resolved target remains within the intended destination. 4. Avoid copying uncontrolled repository contents with `cp -r "$SOURCE_DIR/."`. Instead, copy an explicit allowlist matching the package publication list, such as `SKILL.md`, `LICENSE`, `README.md`, `references`, `scripts`, and required assets. 5. Remove `.git` from the copy source set rather than copying it and deleting it afterward. This avoids an unnecessary destructive command. 6. Create a staging directory outside `SOURCE_DIR`, verify required files such as `SKILL.md`, and atomically rename the completed staging directory into place. 7. Add shell-installer regression tests covering: - A destination equal to `SOURCE_DIR`. - A destination below `SOURCE_DIR`. - Relative paths containing `..`. - Symlinked destination and parent paths. - Existing partial installations. - Paths containing spaces and platform-specific path forms. 8. Keep behavior aligned with the safety validation already present in `bin/install.js`.
