T09 · Insecure Skill Coding Practices
Warning
- Location
create-skills.sh:47- Finding
Unconditional Overwrite of Existing Skill Files in a Privileged Fixed Directory
- Content
View full analysis
"$SKILLS_DIR/$slug/SKILL.md" << 'SKILL_TEMPLATE' ``` ```bash cat > "$SKILLS_DIR/$slug/_meta.json" << META_TEMPLATE ``` ```bash cat > "$WORKFLOWS_DIR/$slug/SKILL.md" << WORKFLOW_TEMPLATE ``` ```bash cat > "$WORKFLOWS_DIR/$slug/_meta.json" << META_TEMPLATE ``` ### Technical Analysis The script writes generated content using the shell truncating-redirection operator (`>`). If a destination already exists, the shell truncates it before writing the replacement content. The script does not: - Check whether the destination already exists. - Request confirmation before replacement. - Create a backup. - Provide an explicit `--force` option. - Enable shell no-clobber behavior. - Validate destination ownership or file type. - Reject symbolic links. - Canonicalize and validate the destination path. The destination is hardcoded beneath `/root`, which means normal users cannot generally run the script successfully. This design encourages execution with elevated privileges even though generating Skill templates does not inherently require root access. If an existing destination is a symbolic link, ordinary shell redirection follows it. Under a deployment where another party can modify entries inside the destination hierarchy, this could cause the privileged process to truncate the link target. Exploitation of that condition requires prior ability to place or replace an entry in the target hierarchy; the reviewed project does not itself grant that ability. ### Attack Path 1. Existing customized or trusted Skill files are present under `/root/.openclaw/workspace/skills/require ...[truncated 1731 chars]- Remediation
View remediation
&2 exit 1 fi ``` 3. **Create destination directories explicitly** ```bash mkdir -p -- "$SKILLS_DIR" "$WORKFLOWS_DIR" ``` 4. **Prevent replacement by default** - Test each destination with `[[ -e "$destination" || -L "$destination" ]]`. - Exit if it already exists. - Require an explicit `--force` option before replacing files. - Alternatively, enable `set -o noclobber` and handle collisions safely. 5. **Reject symbolic links** ```bash if [[ -L "$destination" ]]; then echo "Refusing to overwrite symbolic link: $destination" >&2 exit 1 fi ``` 6. **Back up files before forced replacement** - Create a timestamped backup outside the replacement directory. - Preserve ownership, permissions, and timestamps where appropriate. - Report every replaced file to the user. 7. **Use atomic file creation** - Write content to a temporary file created securely in the destination directory. - Set appropriate permissions. - Rename the temporary file into place only after successful generation and validation. 8. **Validate canonical paths** - Resolve the configured base directory to its canonical form. - Verify every generated destination remains beneath that base. - Reject unexpected traversal, link resolution, or ownership conditions. 9. **Display a dry-run summary** - List files that will be created, skipped, or replaced. - Require confirmation for destructive operations in interactive use. ]]>