T09 · Insecure Skill Coding Practices
Error
- Location
scripts/schedule_sync.sh:36- Finding
Persistent Command Execution Through Unsafe Cron Entry Construction
- Content
View full analysis
> \"$LOG_FILE\" 2>&1" # Check whether a task already exists if crontab -l 2>/dev/null | grep -q "$SYNC_SCRIPT"; then echo -e "${YELLOW}Scheduled task already exists; updating...${NC}" crontab -l 2>/dev/null | grep -v "$SYNC_SCRIPT" | crontab - fi # Add the new task (crontab -l 2>/dev/null; echo "$cron_job") | crontab - } ``` The interval is accepted without validation: ```bash INTERVAL=1 while [[ $# -gt 0 ]]; do case $1 in --interval) INTERVAL="$2" shift 2 ;; ``` ### Technical Analysis The script interpolates the user-controlled `interval` and `directory` values directly into a crontab command. Cron executes task commands through a shell, so quoting the directory with double quotes does not prevent command substitution, variable expansion, or quote termination when the task later runs. The `interval` parameter can also contain spaces or newline characters because it is not restricted to an integer. Such input can alter the cron schedule or introduce an additional cron entry. The directory existence check does not eliminate the issue. Unix paths can contain spaces, quotes, dollar signs, parentheses, and other shell-significant characters. An attacker can create a directory whose literal name contains a command-substitution expression and then register that directory. Sched ...[truncated 1488 chars]- Remediation
View remediation
24 )); then echo "Invalid interval" >&2 exit 1 fi ``` 2. Reject directory values containing newlines, carriage returns, NUL-equivalent input, or other characters that cannot be represented safely in a crontab entry. 3. Do not embed an arbitrary directory directly into a shell command. Store the canonical directory in a user-owned configuration file with mode `0600`, and install a fixed cron command that reads the configuration without evaluating it as shell syntax. 4. If direct insertion is unavoidable, use a rigorously tested shell-quoting routine rather than double quotes alone. 5. Resolve and validate the directory with `realpath`, and ensure the cron wrapper treats the resulting value strictly as data. 6. Add a unique marker to the managed cron entry and modify only that exact entry rather than removing every line that happens to contain the script path. 7. Display the complete proposed cron entry and require explicit confirmation before installation. ]]>