T09 · Insecure Skill Coding Practices
- Location
scripts/script.sh:429- Finding
Persistent Command Injection Through Unsafe Cron Job Construction
- Content
View full analysis
Vulnerability Details
File Location:
scripts/script.sh, lines 429–441
Vulnerability Type: Persistent shell command injection through crontab
Risk Level: HighVulnerable Code
bash local cron_cmd="*/${interval} * * * * bash ${script_path} check '${url}' >> ${DATA_DIR}/cron.log 2>&1" # Check if already scheduled if crontab -l 2>/dev/null | grep -qF "$url"; then echo "⚠️ URL already scheduled. Updating interval..." local temp_cron temp_cron=$(mktemp) crontab -l 2>/dev/null | grep -vF "$url" > "$temp_cron" echo "$cron_cmd" >> "$temp_cron" crontab "$temp_cron" rm -f "$temp_cron" else (crontab -l 2>/dev/null; echo "$cron_cmd") | crontab - fiTechnical Analysis
The
schedulecommand inserts the user-controlledurlandintervalvalues directly into a cron expression. Neither value is validated or safely encoded for the crontab and shell syntaxes.The URL is surrounded by single quotes, but a URL containing a single quote can terminate that quoted argument. Shell operators and commands following it will then be interpreted when cron executes the entry. Newline characters could also introduce additional crontab entries.
The interval is inserted into the cron timing field without confirming that it is a bounded positive integer. Crafted whitespace or newline content could corrupt the schedule or inject another entry.
This flaw is especially severe because the generated payload is written to the user's crontab. The injected command therefore survives the original process and executes periodically across sessions.
The update logic also identifies existing jobs using a substring search:
bash crontab -l 2>/dev/null | grep -vF "$url"This can remove unrelated cron entries that happen to contain the supplied text.
Cron-based scheduling itself is explicitly documented and is reasonably related to periodic website monitoring. However, unsafe construction of the persistent task exceeds a safe i ...[truncated 1233 chars]
- Remediation
View remediation
Remediation Suggestions
-
Validate
intervalusing a strict numeric allowlist and enforce a safe range:bash [[ "$interval" =~ ^[0-9]+$ ]] || { echo "Invalid interval" >&2 return 1 } (( interval >= 1 && interval <= 59 )) || { echo "Interval must be between 1 and 59 minutes" >&2 return 1 } -
Reject URLs containing control characters, especially carriage returns and newlines.
-
Validate that the URL uses an explicitly supported scheme such as
httporhttps. -
Shell-escape every command argument rather than interpolating raw values. For Bash-executed cron commands, construct arguments with
printf '%q':bash printf -v quoted_script '%q' "$script_path" printf -v quoted_url '%q' "$url" printf -v quoted_log '%q' "${DATA_DIR}/cron.log" cron_cmd="*/${interval} * * * * bash ${quoted_script} check ${quoted_url} >> ${quoted_log} 2>&1" -
Add a stable, generated identifier to each managed cron entry and update or remove entries only by that exact identifier. Do not use the URL as a broad substring selector.
-
Consider using a scheduler API or a dedicated wrapper that reads validated arguments from a protected data file, reducing the amount of user input embedded in shell command text.
-
Display the exact proposed schedule and require explicit confirmation before installing persistent tasks in interactive use.
-