T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup_cron.sh:17
- Finding
- Unvalidated Cron Schedule Enables Persistent Command Injection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup_cron.sh:17-31` **Vulnerability Type**: Cron command injection through unvalidated configuration **Risk Level**: High ### Vulnerable Code ```bash SCHEDULE=$(python3 - <<PY import json from pathlib import Path config = json.loads(Path(r"$CONFIG_FILE").read_text(encoding="utf-8")) print(config.get("schedule", "0 8 * * *")) PY ) mkdir -p "$LOG_DIR" CRON_MARKER="# btc-monitor-skill" CRON_JOB="$SCHEDULE cd $ROOT_DIR && /usr/bin/env python3 scripts/monitor.py >> $LOG_DIR/monitor.log 2>&1 $CRON_MARKER" (crontab -l 2>/dev/null | grep -v "$CRON_MARKER" || true; echo "$CRON_JOB") | crontab - ``` ### Technical Analysis The script reads the `schedule` property from `config.json` and inserts it directly at the beginning of a crontab entry. It does not verify that the value: - Is a string containing only one line. - Contains exactly the expected cron scheduling fields. - Does not contain an embedded command after the scheduling fields. - Does not contain carriage returns, newline characters, NUL bytes, or other control characters. - Does not use cron directives or special syntax outside the supported schedule format. A crontab line consists of scheduling fields followed by an arbitrary shell command. Consequently, an attacker-controlled schedule can place a command immediately after valid scheduling fields, causing the intended monitor command to become additional shell syntax. Newline characters can also create entirely separate cron entries. For example, a malicious configuration could use a value conceptually equivalent to: ```json { "schedule": "* * * * * attacker_command #" } ``` After concatenation, the resulting entry would execute `attacker_command`, while the comment character prevents the intended monitoring command from affecting execution. A multiline value could install additional independent scheduled jobs. The exploit requires the attacker to influence `config.json` before a u ...[truncated 1731 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Require `schedule` to be a string and reject empty or excessively long values. 2. Reject carriage returns, newline characters, NUL bytes, and all other control characters. 3. Parse the schedule into an explicitly supported format rather than concatenating arbitrary text into a crontab line. 4. If only conventional five-field cron expressions are supported, require exactly five validated fields before constructing the command. 5. Validate each field against an allowlist of supported cron tokens and numeric ranges. Prefer a maintained cron-expression parser over a custom regular expression. 6. Reject cron directives such as `@reboot`, environment assignments, comments, and embedded command text unless deliberately supported. 7. Shell-quote `ROOT_DIR` and `LOG_DIR` when generating the command so spaces and shell metacharacters in paths cannot alter execution. 8. Write the generated entry using controlled formatting rather than interpolating an unrestricted configuration value. 9. Display the exact proposed cron entry and request explicit confirmation before installation. 10. Document and provide an uninstall command that removes the marked entry. A safer validation flow should fail closed before calling `crontab`, for example: ```python schedule = config.get("schedule", "0 8 * * *") if not isinstance(schedule, str): raise SystemExit("schedule must be a string") if any(character in schedule for character in ("\r", "\n", "\0")): raise SystemExit("schedule must contain exactly one line") fields = schedule.split() if len(fields) != 5: raise SystemExit("schedule must be a five-field cron expression") # Validate every field with a trusted cron parser before printing it. ``` The shell script should only install the entry after the validated parser returns a canonical schedule. ]]>