T09 · Insecure Skill Coding Practices
Error
- Location
scripts/setup_vps_retention.sh:7- Finding
Root Cron Command Injection Through an Unvalidated Retention Argument
- Content
View full analysis
/dev/null | grep -v '/srv/reolink/incoming -type f -mtime' ; \ echo "30 3 * * * find /srv/reolink/incoming -type f -mtime +${DAYS} -delete" ) | crontab - ``` ### Technical Analysis The script is intended to run with `sudo` and therefore normally modifies root's crontab. The user-controlled `DAYS` argument is interpolated directly into a cron command without verifying that it is a positive integer. Although the variable is quoted while being passed to `echo`, quoting only protects the setup script's current shell. The generated text is subsequently stored in a crontab and interpreted by `/bin/sh` when cron executes it. Shell metacharacters embedded in `DAYS`, including semicolons, command substitutions, comments, or newline characters, therefore become executable syntax in the persistent root cron job. For example, a value conceptually shaped like the following would add a second shell operation to the generated cron command: ```text 30; # ``` The resulting cron entry would resemble: ```cron 30 3 * * * find /srv/reolink/incoming -type f -mtime +30; # -delete ``` The scheduled attacker command would then execute with the privileges of the crontab owner, normally root. ### Attack Path 1. An attacker influences the argument passed as the retention period, such as through an administrative wrapper, copied command, automation variable, or social-engineering instruction. 2. An administrator invokes the documented command with `sudo`. 3. The script accepts the malicious value without validation. 4. The value is incorporated into a root crontab entry as shell syntax. 5. At 03:30, cron executes the injected operation as root. 6. Because the entry persists in root's ...[truncated 577 chars]- Remediation
View remediation
3650 )); then echo "ERROR: retention days must be an integer between 1 and 3650" >&2 exit 1 fi ``` Additional hardening should include: 1. Prefer a fixed root-owned cleanup script whose arguments are validated internally, with cron invoking only that fixed path. 2. Use a systemd service and timer with a static `ExecStart` instead of dynamically generating shell commands. 3. Install the cron entry in `/etc/cron.d/` as a root-owned file with restrictive permissions rather than rewriting the invoking user's entire crontab. 4. Verify the generated configuration before installation. 5. Avoid printing the complete root crontab, since unrelated entries may contain sensitive operational information. 6. Provide an explicit uninstall procedure for the retention schedule. ]]>