T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:98
- Finding
- Persistent Command Injection Through Unsafely Generated Cron Entries## Vulnerability Details **File Location**: `SKILL.md:98-101` **Vulnerability Type**: Shell command injection in persistent scheduled tasks **Risk Level**: High ### Vulnerable Code ```bash # Signal scan: every 15 minutes (crontab -l 2>/dev/null; echo "*/15 * * * * USER_ID=$TELEGRAM_USER_ID python3 $SKILL_PATH/scripts/signal_cron.py $STRATEGY_ID >> $SKILL_PATH/state/$TELEGRAM_USER_ID.$STRATEGY_ID.log 2>&1") | crontab - # Performance review: daily at 09:00 UTC (crontab -l 2>/dev/null; echo "0 9 * * * USER_ID=$TELEGRAM_USER_ID python3 $SKILL_PATH/scripts/review_cron.py $STRATEGY_ID >> $SKILL_PATH/state/$TELEGRAM_USER_ID.review.log 2>&1") | crontab - ``` ### Technical Analysis The cron instructions interpolate `TELEGRAM_USER_ID`, `SKILL_PATH`, and `STRATEGY_ID` directly into shell and crontab syntax without validation or safe quoting. A strategy ID may be supplied through the strategy configuration, and the implementation does not restrict its characters. Shell metacharacters in any interpolated value can change the command executed by cron. A newline can additionally terminate the current cron record and inject another scheduled entry. Because the resulting command is installed into the user's crontab, exploitation persists across sessions and repeatedly executes with the privileges of the account that installed the job. The use of scheduled execution is relevant to the declared automated market-scanning functionality and the documentation requires user confirmation before installation. The persistence mechanism therefore has a legitimate purpose, but its unsafe construction exceeds what is necessary and turns untrusted identifiers into a persistent code-execution channel. ### Attack Path 1. An attacker influences a strategy configuration so that its explicit `id` contains shell syntax or a newline followed by another cron entry. 2. The strategy is created, and its attacker-controlled ID is returned as `STRATEGY_ID ...[truncated 938 chars]
- Remediation
- ## Remediation Suggestions 1. Restrict user and strategy identifiers to a conservative allowlist such as `^[A-Za-z0-9_-]+$`. 2. Reject newlines, whitespace, path separators, shell metacharacters, and leading hyphens. 3. Resolve `SKILL_PATH` to a trusted absolute path and verify that it points to the installed Skill directory. 4. Avoid composing cron records through interpolated shell strings. Generate a fixed wrapper script whose arguments are loaded from a validated configuration file. 5. If arguments must appear in a cron record, quote them using a robust shell-quoting mechanism and separately reject newline characters because shell quoting does not safely delimit cron records. 6. Install cron entries with unique markers, detect duplicates, and provide an explicit removal command. 7. Display the exact generated cron records and request confirmation immediately before installation. 8. Prefer a scheduler interface that accepts an argument array rather than invoking a shell.