T09 · Insecure Skill Coding Practices
Warning
- Location
- install.sh:62
- Finding
- Unsafe Recursive Deletion of an Existing Skill Directory## Vulnerability Details **File Location**: `install.sh`, lines 62–66 **Vulnerability Type**: Unsafe recursive deletion during installation **Risk Level**: Medium ```bash LINK_TARGET="${SKILL_DIR}/${BIN_NAME}" if [ -L "${LINK_TARGET}" ] || [ -d "${LINK_TARGET}" ]; then rm -rf "${LINK_TARGET}" fi ``` ### Technical Analysis The installer deletes an existing path at `~/.openclaw/workspace/skills/openrouter-usage` before creating the Skill symlink. The condition permits both symbolic links and real directories, while `rm -rf` recursively removes either without distinguishing between them. Although the user is asked whether to link the Skill, the prompt does not disclose that accepting the default action may permanently erase an existing directory and all files beneath it. Recursive deletion of a real directory is not necessary to replace an existing symbolic link and violates safe installation practices. The destination is constructed from fixed variables, so the reviewed code does not expose direct shell command injection. Nevertheless, any pre-existing data at the fixed destination can be destroyed with the permissions of the user running the installer. ### Attack Path 1. A real directory exists at `~/.openclaw/workspace/skills/openrouter-usage`. 2. The directory contains an existing Skill installation, local modifications, or other user-created files. 3. The user runs `install.sh`. 4. The installer asks whether to link the project as an OpenClaw workspace Skill, defaulting to `Y`. 5. The user accepts the prompt without being warned that existing content will be recursively deleted. 6. The `rm -rf "${LINK_TARGET}"` command permanently removes the existing directory tree. 7. The deleted directory is replaced with a symbolic link to the current project. ### Impact Assessment Exploitation does not grant additional privileges or code execution beyond those already held by the installer process. Its primary im ...[truncated 413 chars]
- Remediation
- ## Remediation Suggestions 1. Treat symbolic links and real directories separately. 2. Remove an existing symbolic link with non-recursive `rm -- "${LINK_TARGET}"`. 3. Refuse to overwrite a real directory by default. 4. If directory replacement is required, display the exact path and require a separate explicit confirmation. 5. Back up or rename an existing directory instead of permanently deleting it. 6. Validate that the destination remains the expected direct child of `${SKILL_DIR}` before modifying it. 7. Use `ln -s` only after confirming that the destination no longer exists. A safer implementation would resemble: ```bash if [ -L "${LINK_TARGET}" ]; then rm -- "${LINK_TARGET}" elif [ -e "${LINK_TARGET}" ]; then echo -e "${RED}Error: Refusing to overwrite existing path: ${LINK_TARGET}${NC}" echo "Move or back up that path, then rerun the installer." exit 1 fi ln -s -- "${SCRIPT_DIR}" "${LINK_TARGET}" ```