T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/install.sh:13
- Finding
- Cron command injection through insufficient check_time validation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/install.sh`, lines 13–24 and 37–40 **Vulnerability Type**: Persistent command injection through generated crontab content **Risk Level**: High ### Vulnerable Code ```sh CHECK_TIME=$(/usr/bin/python3 - <<'PY' "$CONFIG_PATH" import json,sys cfg=json.load(open(sys.argv[1])) print(cfg.get('check_time','22:20')) PY ) HOUR="${CHECK_TIME%%:*}" MIN="${CHECK_TIME##*:}" if [[ -z "$HOUR" || -z "$MIN" ]]; then echo "Invalid check_time in config.json, expected HH:MM" exit 1 fi ( crontab -l 2>/dev/null || true ) | sed '/calendar_clean_notify.sh/d' > "$TMPCRON" echo "$MIN $HOUR * * * CAL_SKILL_CONFIG=$CONFIG_PATH $SCRIPT_DIR/calendar_clean_notify.sh" >> "$TMPCRON" crontab "$TMPCRON" ``` ### Technical Analysis The installer reads `check_time` from `config.json` and inserts the resulting values directly into a crontab file. Validation only verifies that the derived hour and minute strings are nonempty. It does not: - Require numeric values. - Enforce hour and minute ranges. - Reject newline or carriage-return characters. - Reject shell or cron metacharacters. - Safely encode `CONFIG_PATH` or `SCRIPT_DIR` for execution by cron's shell. A crafted configuration value can therefore inject additional crontab records or alter the shell command installed by the Skill. Since crontab entries survive the installer process, successful exploitation results in persistent execution. The scheduled duplicate-check feature is documented and functionally relevant, but unsafe generation of the crontab entry exceeds what is necessary to implement it securely. ### Attack Path 1. An attacker gains the ability to modify or supply the project's `config.json`, including through an untrusted project archive or configuration automation. 2. The attacker places newline-delimited cron content or shell syntax in `check_time`. 3. The user or Agent runs `scripts/install.sh`. 4. The installer writes the malicious value into the tempor ...[truncated 639 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Validate `check_time` with a strict expression such as `^(?:[01][0-9]|2[0-3]):[0-5][0-9]$`. - Reject all control characters, including newlines and carriage returns. - Convert the validated hour and minute to integers before generating the schedule. - Safely quote the configuration and script paths for the shell used by cron. - Prefer installing a fixed wrapper script with no configuration-derived shell syntax. - Consider a macOS LaunchAgent whose `ProgramArguments` are represented as an argument array rather than a shell command. - Show the exact scheduled entry and require explicit confirmation before installation. ]]>