T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/backup-generator.sh:35
- Finding
- Unvalidated backup parameters enable generated-script and cron command injection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/backup-generator.sh`, lines 35-43, 72-99, and 467-470 **Vulnerability Type**: Shell and crontab command injection **Risk Level**: High ### Vulnerable Code ```bash while [[ $# -gt 0 ]]; do case "$1" in --type) BACKUP_TYPE="$2"; shift 2 ;; --target) TARGET="$2"; shift 2 ;; --dest) DESTINATION="$2"; shift 2 ;; --schedule) SCHEDULE="$2"; shift 2 ;; --encrypt) ENCRYPT=true; shift ;; --retain) RETAIN_DAYS="$2"; shift 2 ;; --output-dir) OUTPUT_DIR="$2"; shift 2 ;; --notify) NOTIFY_TYPE="$2"; shift 2 ;; ``` ```bash SCRIPT_NAME="backup_${BACKUP_TYPE}_$(echo "$TARGET" | tr '/' '_' | tr '.' '_').sh" SCRIPT_PATH="${OUTPUT_DIR}/${SCRIPT_NAME}" ``` ```bash echo "BACKUP_TYPE=\"${BACKUP_TYPE}\"" echo "TARGET=\"${TARGET}\"" echo "DESTINATION=\"${DESTINATION}\"" echo "RETAIN_DAYS=\"\${RETAIN_DAYS:-${RETAIN_DAYS}}\"" echo "ENCRYPT=\"${ENCRYPT}\"" ``` ```bash read -rp "是否添加到 crontab？(y/N): " add_cron if [ "$add_cron" = "y" ] || [ "$add_cron" = "Y" ]; then (crontab -l 2>/dev/null | grep -v "$SCRIPT_PATH"; echo "$SCHEDULE $SCRIPT_PATH >> ${LOG_FILE} 2>&1") | crontab - echo -e "${GREEN}[✓]${NC} cron 任务已添加" fi ``` ### Technical Analysis The generator accepts backup type, target, destination, schedule, retention, output directory, and notification type as arbitrary strings. Several values are interpolated directly into executable shell source inside double-quoted assignments. Embedded quotes, command substitutions, shell metacharacters, or newline characters can break out of the intended assignment and add commands to the generated backup script. The cron schedule and generated script path are also inserted directly into crontab text. In particular, a newline in `SCHEDULE` or `SCRIPT_PATH` can create an additional cron entry. The resulting payload persists and is executed by cron with the privileges of the user wh ...[truncated 1234 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Reject carriage returns, newlines, null bytes, quotes, backticks, and other shell-control characters in all generated values. - Restrict `BACKUP_TYPE` and `NOTIFY_TYPE` to explicit allowlists. - Validate `RETAIN_DAYS` as a bounded positive integer. - Parse and validate the cron expression as exactly five permitted fields. - Serialize values written into shell source with `printf '%q'` rather than manual quotation. - Prefer a static backup program with a non-executable configuration file instead of generating shell source. - Canonicalize and restrict `OUTPUT_DIR` to an approved directory. - Create cron entries through a dedicated file or structured scheduler interface, using fixed commands and validated arguments. - Display the exact final generated script and exact resulting crontab diff before obtaining confirmation. - Use unique identifiers or comments to replace only the intended cron entry instead of matching an untrusted path with `grep`. ]]>